Parcourir la source

Statistics data transfer method is changed
from peliodic sending via statistics channel
to passive retrieval via config session

- Added "getstats" command to retrieve statistics data via config session.
= type "Auth getstats" command using bindctl shows statistics data.
- Removed StatisticsIntervalTimer
- Removed "sendstats" command
- Changed that each config command can return values.
- returns Element::createlist(), Element::fromJSON("{}") or Statistics

Opinion:
- Each command should return conditions and values using createAnswer()
rather than using throw/catch.

Kazunori Fujiwara il y a 12 ans
Parent
commit
a580674f5a

+ 2 - 2
src/bin/auth/auth.spec.pre.in

@@ -110,8 +110,8 @@
         ]
       },
       {
-        "command_name": "sendstats",
-        "command_description": "Send data to a statistics module at once",
+        "command_name": "getstats",
+        "command_description": "Retrieve statistics data",
         "command_args": []
       },
       {

+ 1 - 34
src/bin/auth/auth_config.cc

@@ -53,37 +53,6 @@ public:
     virtual void commit() {};
 };
 
-/// A derived \c AuthConfigParser class for the "statistics-internal"
-/// configuration identifier.
-class StatisticsIntervalConfig : public AuthConfigParser {
-public:
-    StatisticsIntervalConfig(AuthSrv& server) :
-        server_(server), interval_(0)
-    {}
-    virtual void build(ConstElementPtr config_value) {
-        const int32_t config_interval = config_value->intValue();
-        if (config_interval < 0) {
-            isc_throw(AuthConfigError, "Negative statistics interval value: "
-                      << config_interval);
-        }
-        if (config_interval > 86400) {
-            isc_throw(AuthConfigError, "Statistics interval value "
-                      << config_interval
-                      << " must be equal to or shorter than 86400");
-        }
-        interval_ = config_interval;
-    }
-    virtual void commit() {
-        // setStatisticsTimerInterval() is not 100% exception free.  But
-        // exceptions should happen only in a very rare situation, so we
-        // let them be thrown and subsequently regard them as a fatal error.
-        server_.setStatisticsTimerInterval(interval_);
-    }
-private:
-    AuthSrv& server_;
-    uint32_t interval_;
-};
-
 /// A special parser for testing: it throws from commit() despite the
 /// suggested convention of the class interface.
 class ThrowerCommitConfig : public AuthConfigParser {
@@ -155,9 +124,7 @@ createAuthConfigParser(AuthSrv& server, const std::string& config_id) {
     // simplicity.  In future we'll probably generalize it using map-like
     // data structure, and may even provide external register interface so
     // that it can be dynamically customized.
-    if (config_id == "statistics-interval") {
-        return (new StatisticsIntervalConfig(server));
-    } else if (config_id == "listen_on") {
+    if (config_id == "listen_on") {
         return (new ListenAddressConfig(server));
     } else if (config_id == "_commit_throw") {
         // This is for testing purpose only and should not appear in the

+ 2 - 38
src/bin/auth/auth_srv.cc

@@ -247,9 +247,6 @@ public:
     ModuleCCSession* config_session_;
     AbstractSession* xfrin_session_;
 
-    /// Interval timer for periodic submission of statistics counters.
-    IntervalTimer statistics_timer_;
-
     /// Query counters for statistics
     AuthCounters counters_;
 
@@ -320,7 +317,6 @@ AuthSrvImpl::AuthSrvImpl(AbstractXfroutClient& xfrout_client,
                          BaseSocketSessionForwarder& ddns_forwarder) :
     config_session_(NULL),
     xfrin_session_(NULL),
-    statistics_timer_(io_service_),
     counters_(),
     keyring_(NULL),
     ddns_base_forwarder_(ddns_forwarder),
@@ -479,43 +475,11 @@ AuthSrv::setConfigSession(ModuleCCSession* config_session) {
     impl_->registerStatisticsValidator();
 }
 
-void
-AuthSrv::setStatisticsSession(AbstractSession* statistics_session) {
-    impl_->counters_.setStatisticsSession(statistics_session);
-}
-
 ModuleCCSession*
 AuthSrv::getConfigSession() const {
     return (impl_->config_session_);
 }
 
-uint32_t
-AuthSrv::getStatisticsTimerInterval() const {
-    return (impl_->statistics_timer_.getInterval() / 1000);
-}
-
-void
-AuthSrv::setStatisticsTimerInterval(uint32_t interval) {
-    if (interval == impl_->statistics_timer_.getInterval()) {
-        return;
-    }
-    if (interval > 86400) {
-        // It can't occur since the value is checked in
-        // statisticsIntervalConfig::build().
-        isc_throw(InvalidParameter, "Too long interval: " << interval);
-    }
-    if (interval == 0) {
-        impl_->statistics_timer_.cancel();
-        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_STATS_TIMER_DISABLED);
-    } else {
-        impl_->statistics_timer_.setup(boost::bind(&AuthSrv::submitStatistics,
-                                                   this),
-                                       interval * 1000);
-        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_STATS_TIMER_SET)
-                  .arg(interval);
-    }
-}
-
 void
 AuthSrv::processMessage(const IOMessage& io_message, Message& message,
                         OutputBuffer& buffer, DNSServer* server)
@@ -882,8 +846,8 @@ AuthSrv::updateConfig(ConstElementPtr new_config) {
     }
 }
 
-bool AuthSrv::submitStatistics() const {
-    return (impl_->counters_.submitStatistics());
+ElementPtr AuthSrv::getStatistics() const {
+    return (impl_->counters_.getStatistics());
 }
 
 uint64_t

+ 6 - 31
src/bin/auth/auth_srv.h

@@ -17,7 +17,6 @@
 
 #include <string>
 
-#include <cc/data.h>
 #include <config/ccsession.h>
 #include <datasrc/factory.h>
 #include <dns/message.h>
@@ -222,45 +221,21 @@ public:
     /// is shutdown.
     void setStatisticsSession(isc::cc::AbstractSession* statistics_session);
 
-    /// Return the interval of periodic submission of statistics in seconds.
-    ///
-    /// If the statistics submission is disabled, it returns 0.
-    ///
-    /// This method never throws an exception.
-    uint32_t getStatisticsTimerInterval() const;
-
-    /// Set the interval of periodic submission of statistics.
-    ///
-    /// If the specified value is non 0, the \c AuthSrv object will submit
-    /// its statistics to the statistics module every \c interval seconds.
-    /// If it's 0, and \c AuthSrv currently submits statistics, the submission
-    /// will be disabled. \c interval must be equal to or shorter than 86400
-    /// seconds (1 day).
-    ///
-    /// This method should normally not throw an exception; however, its
-    /// underlying library routines may involve resource allocation, and
-    /// when it fails it would result in a corresponding standard exception.
-    ///
-    /// \param interval The submission interval in seconds if non 0;
-    /// or a value of 0 to disable the submission.
-    void setStatisticsTimerInterval(uint32_t interval);
-
-    /// \brief Submit statistics counters to statistics module.
+    /// \brief Returns statistics data
     ///
     /// This function can throw an exception from
-    /// AuthCounters::submitStatistics().
+    /// AuthCounters::getStatistics().
     ///
-    /// \return true on success, false on failure (e.g. session timeout,
-    /// session error).
-    bool submitStatistics() const;
+    /// \return JSON format statistics data.
+    isc::data::ElementPtr getStatistics() const;
 
     /// \brief Get the value of counter in the AuthCounters.
     ///
-    /// This function calls AuthCounters::getCounter() and
+    /// This function calls AuthCounters::getStatistics() and
     /// returns its return value.
     ///
     /// This function never throws an exception as far as
-    /// AuthCounters::getCounter() doesn't throw.
+    /// AuthCounters::getStatistics() doesn't throw.
     ///
     /// Note: Currently this function is for testing purpose only.
     ///

+ 24 - 18
src/bin/auth/command.cc

@@ -100,14 +100,14 @@ public:
     ///
     /// \param server The \c AuthSrv object on which the command is executed.
     /// \param args Command specific argument.
-    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr args) = 0;
+    /// \return command result data in JSON format.
+    virtual ElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr args) = 0;
 };
 
-// Handle the "shutdown" command. An optional parameter "pid" is used to
-// see if it is really for our instance.
+// Handle the "shutdown" command.
 class ShutdownCommand : public AuthCommand {
 public:
-    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr args) {
+    virtual ElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr args) {
         // Is the pid argument provided?
         if (args && args->contains("pid")) {
             // If it is, we check it is the same as our PID
@@ -123,41 +123,44 @@ public:
                 // there are multiple instances of the server running and
                 // another instance is being shut down, we get the message
                 // too, due to the multicast nature of our message bus.
-                return;
+                return (Element::createList());
             }
         }
         LOG_DEBUG(auth_logger, DBG_AUTH_SHUT, AUTH_SHUTDOWN);
         server.stop();
+        return (Element::createList());
     }
 };
 
-// Handle the "sendstats" command.  No argument is assumed.
-class SendStatsCommand : public AuthCommand {
+// Handle the "getstats" command.  The argument is a list.
+class GetStatsCommand : public AuthCommand {
 public:
-    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr) {
+    virtual ElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr) {
         LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_RECEIVED_SENDSTATS);
-        server.submitStatistics();
+        return (server.getStatistics());
     }
 };
 
 class StartDDNSForwarderCommand : public AuthCommand {
 public:
-    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr) {
+    virtual ElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr) {
         server.createDDNSForwarder();
+        return (Element::fromJSON("{}"));
     }
 };
 
 class StopDDNSForwarderCommand : public AuthCommand {
 public:
-    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr) {
+    virtual ElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr) {
         server.destroyDDNSForwarder();
+        return (Element::fromJSON("{}"));
     }
 };
 
 // Handle the "loadzone" command.
 class LoadZoneCommand : public AuthCommand {
 public:
-    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr args) {
+    virtual ElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr args) {
         if (args == NULL) {
             isc_throw(AuthCommandError, "Null argument");
         }
@@ -185,7 +188,7 @@ public:
                 // Everything worked fine.
                 LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_LOAD_ZONE)
                     .arg(zone_class).arg(origin);
-                return;
+                return (Element::createList());
             case ConfigurableClientList::ZONE_NOT_FOUND:
                 isc_throw(AuthCommandError, "Zone " << origin << "/" <<
                           zone_class << " was not found in any configured "
@@ -202,6 +205,7 @@ public:
                 isc_throw(isc::Unexpected, "Cache disabled in client list of "
                           "class " << zone_class);
         }
+        return (Element::createList());
     }
 };
 
@@ -212,8 +216,8 @@ createAuthCommand(const string& command_id) {
     // (see also createAuthConfigParser())
     if (command_id == "shutdown") {
         return (new ShutdownCommand());
-    } else if (command_id == "sendstats") {
-        return (new SendStatsCommand());
+    } else if (command_id == "getstats") {
+        return (new GetStatsCommand());
     } else if (command_id == "loadzone") {
         return (new LoadZoneCommand());
     } else if (command_id == "start_ddns_forwarder") {
@@ -236,15 +240,17 @@ ConstElementPtr
 execAuthServerCommand(AuthSrv& server, const string& command_id,
                       ConstElementPtr args)
 {
+    ElementPtr value;
+
     LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_RECEIVED_COMMAND).arg(command_id);
     try {
-        scoped_ptr<AuthCommand>(createAuthCommand(command_id))->exec(server,
-                                                                     args);
+        value = scoped_ptr<AuthCommand>(createAuthCommand(command_id))->exec(server,
+                                                                             args);
     } catch (const isc::Exception& ex) {
         LOG_ERROR(auth_logger, AUTH_COMMAND_FAILED).arg(command_id)
                                                    .arg(ex.what());
         return (createAnswer(1, ex.what()));
     }
 
-    return (createAnswer());
+    return (createAnswer(0, value));
 }

+ 0 - 14
src/bin/auth/main.cc

@@ -123,9 +123,7 @@ main(int argc, char* argv[]) {
     // XXX: we should eventually pass io_service here.
     Session* cc_session = NULL;
     Session* xfrin_session = NULL;
-    Session* statistics_session = NULL;
     bool xfrin_session_established = false; // XXX (see Trac #287)
-    bool statistics_session_established = false; // XXX (see Trac #287)
     ModuleCCSession* config_session = NULL;
     XfroutClient xfrout_client(getXfroutSocketPath());
     SocketSessionForwarder ddns_forwarder(getDDNSSocketPath());
@@ -173,14 +171,7 @@ main(int argc, char* argv[]) {
         xfrin_session_established = true;
         LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_XFRIN_CHANNEL_ESTABLISHED);
 
-        statistics_session = new Session(io_service.get_io_service());
-        LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_STATS_CHANNEL_CREATED);
-        statistics_session->establish(NULL);
-        statistics_session_established = true;
-        LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_STATS_CHANNEL_ESTABLISHED);
-
         auth_server->setXfrinSession(xfrin_session);
-        auth_server->setStatisticsSession(statistics_session);
 
         // Configure the server.  configureAuthServer() is expected to install
         // all initial configurations, but as a short term workaround we
@@ -226,16 +217,11 @@ main(int argc, char* argv[]) {
         ret = 1;
     }
 
-    if (statistics_session_established) {
-        statistics_session->disconnect();
-    }
-
     if (xfrin_session_established) {
         xfrin_session->disconnect();
     }
 
     DataSourceConfigurator::cleanup();
-    delete statistics_session;
     delete xfrin_session;
     delete config_session;
     delete cc_session;

+ 14 - 61
src/bin/auth/statistics.cc

@@ -55,8 +55,7 @@ public:
     }
     void inc(const std::string& zone,
              const AuthCounters::PerZoneCounterType type);
-    bool submitStatistics() const;
-    void setStatisticsSession(isc::cc::AbstractSession* statistics_session);
+    isc::data::ElementPtr getStatistics() const;
     void registerStatisticsValidator
     (AuthCounters::validator_type validator);
     // Currently for testing purpose only
@@ -74,7 +73,6 @@ private:
     Counter rcode_counter_;
     static const size_t NUM_RCODES = 17;
     CounterDictionary per_zone_counter_;
-    isc::cc::AbstractSession* statistics_session_;
     AuthCounters::validator_type validator_;
 };
 
@@ -84,8 +82,7 @@ AuthCountersImpl::AuthCountersImpl() :
     // size of per_zone_counter_: AuthCounters::PER_ZONE_COUNTER_TYPES
     server_counter_(AuthCounters::SERVER_COUNTER_TYPES),
     opcode_counter_(NUM_OPCODES), rcode_counter_(NUM_RCODES),
-    per_zone_counter_(AuthCounters::PER_ZONE_COUNTER_TYPES),
-    statistics_session_(NULL)
+    per_zone_counter_(AuthCounters::PER_ZONE_COUNTER_TYPES)
 {
     per_zone_counter_.addElement("_SERVER_");
 }
@@ -105,20 +102,10 @@ AuthCountersImpl::inc(const std::string& zone,
     per_zone_counter_[zone].inc(type);
 }
 
-bool
-AuthCountersImpl::submitStatistics() const {
-    if (statistics_session_ == NULL) {
-        LOG_ERROR(auth_logger, AUTH_NO_STATS_SESSION);
-        return (false);
-    }
+isc::data::ElementPtr
+AuthCountersImpl::getStatistics() const {
     std::stringstream statistics_string;
-    // add pid in order for stats to identify which auth sends
-    // statistics in the situation that multiple auth instances are
-    // working
-    statistics_string << "{\"command\": [\"set\","
-                      <<   "{ \"owner\": \"Auth\","
-                      <<   "  \"pid\":" << getpid()
-                      <<   ", \"data\":"
+    statistics_string 
                       <<     "{ \"queries.udp\": "
                       <<     server_counter_.get(AuthCounters::SERVER_UDP_QUERY)
                       <<     ", \"queries.tcp\": "
@@ -150,45 +137,18 @@ AuthCountersImpl::submitStatistics() const {
                               << counter;
         }
     }
-    statistics_string <<   " }"
-                      <<   "}"
-                      << "]}";
-    isc::data::ConstElementPtr statistics_element =
+    statistics_string <<   "}";
+
+    isc::data::ElementPtr statistics_element =
         isc::data::Element::fromJSON(statistics_string);
     // validate the statistics data before send
     if (validator_) {
-        if (!validator_(
-                statistics_element->get("command")->get(1)->get("data"))) {
+        if (!validator_(statistics_element)) {
             LOG_ERROR(auth_logger, AUTH_INVALID_STATISTICS_DATA);
-            return (false);
+            return (isc::data::ElementPtr());
         }
     }
-    try {
-        // group_{send,recv}msg() can throw an exception when encountering
-        // an error, and group_recvmsg() will throw an exception on timeout.
-        // We don't want to kill the main server just due to this, so we
-        // handle them here.
-        const int seq =
-            statistics_session_->group_sendmsg(statistics_element, "Stats");
-        isc::data::ConstElementPtr env, answer;
-        // TODO: parse and check response from statistics module
-        // currently it just returns empty message
-        statistics_session_->group_recvmsg(env, answer, false, seq);
-    } catch (const isc::cc::SessionError& ex) {
-        LOG_ERROR(auth_logger, AUTH_STATS_COMMS).arg(ex.what());
-        return (false);
-    } catch (const isc::cc::SessionTimeout& ex) {
-        LOG_ERROR(auth_logger, AUTH_STATS_TIMEOUT).arg(ex.what());
-        return (false);
-    }
-    return (true);
-}
-
-void
-AuthCountersImpl::setStatisticsSession
-    (isc::cc::AbstractSession* statistics_session)
-{
-    statistics_session_ = statistics_session;
+    return (statistics_element);
 }
 
 void
@@ -224,16 +184,9 @@ AuthCounters::inc(const Rcode rcode) {
     impl_->inc(rcode);
 }
 
-bool
-AuthCounters::submitStatistics() const {
-    return (impl_->submitStatistics());
-}
-
-void
-AuthCounters::setStatisticsSession
-    (isc::cc::AbstractSession* statistics_session)
-{
-    impl_->setStatisticsSession(statistics_session);
+isc::data::ElementPtr
+AuthCounters::getStatistics() const {
+    return (impl_->getStatistics());
 }
 
 uint64_t

+ 7 - 35
src/bin/auth/statistics.h

@@ -17,8 +17,9 @@
 
 #include <dns/opcode.h>
 #include <dns/rcode.h>
-
 #include <cc/session.h>
+#include <cc/data.h>
+
 #include <stdint.h>
 #include <boost/scoped_ptr.hpp>
 
@@ -39,9 +40,8 @@ class AuthCountersImpl;
 /// Call \c inc() to increment a counter for specific type of query in
 /// the query processing function. use \c enum \c CounterType to specify
 /// the type of query.
-/// Call \c submitStatistics() to submit statistics information to statistics
-/// module with statistics_session, periodically or at a time the command
-/// \c sendstats is received.
+/// Call \c getStatistics() to answer statistics information to statistics
+/// module with statistics_session, when the command \c getstats is received.
 ///
 /// We may eventually want to change the structure to hold values that are
 /// not counters (such as concurrent TCP connections), or seperate generic
@@ -108,43 +108,15 @@ public:
     /// \throw None
     void inc(const isc::dns::Rcode rcode);
 
-    /// \brief Submit statistics counters to statistics module.
-    ///
-    /// This method is desinged to be called periodically
-    /// with \c asio_link::StatisticsSendTimer, or arbitrary
-    /// by the command 'sendstats'.
-    ///
-    /// Note: Set the session to communicate with statistics module
-    /// by \c setStatisticsSession() before calling \c submitStatistics().
+    /// \brief Answers statistics counters to statistics module.
     ///
     /// This method is mostly exception free (error conditions are
     /// represented via the return value). But it may still throw
     /// a standard exception if memory allocation fails inside the method.
     ///
-    /// \return true on success, false on error.
-    ///
-    /// \todo Do not block message handling in auth_srv while submitting
-    /// statistics data.
-    ///
-    bool submitStatistics() const;
-
-    /// \brief Set the session to communicate with Statistics
-    /// module.
-    ///
-    /// This method never throws an exception.
-    ///
-    /// Note: this interface is tentative.  We'll revisit the ASIO and session
-    /// frameworks, at which point the session will probably be passed on
-    /// construction of the server.
-    ///
-    /// Ownership isn't transferred: the caller is responsible for keeping
-    /// this object to be valid while the server object is working and for
-    /// disconnecting the session and destroying the object when the server
-    /// is shutdown.
-    ///
-    /// \param statistics_session A pointer to the session
+    /// \return statistics data
     ///
-    void setStatisticsSession(isc::cc::AbstractSession* statistics_session);
+    isc::data::ElementPtr getStatistics() const;
 
     /// \brief Get the value of a counter in the AuthCounters.
     ///

+ 0 - 2
src/bin/auth/tests/auth_srv_unittest.cc

@@ -100,7 +100,6 @@ protected:
     {
         server.setDNSService(dnss_);
         server.setXfrinSession(&notify_session);
-        server.setStatisticsSession(&statistics_session);
         server.createDDNSForwarder();
     }
 
@@ -180,7 +179,6 @@ protected:
     }
 
     MockDNSService dnss_;
-    MockSession statistics_session;
     MockXfroutClient xfrout;
     MockSocketSessionForwarder ddns_forwarder;
     AuthSrv server;

+ 10 - 12
src/bin/auth/tests/command_unittest.cc

@@ -67,16 +67,13 @@ protected:
         rcode_(-1),
         expect_rcode_(0),
         itimer_(server_.getIOService())
-    {
-        server_.setStatisticsSession(&statistics_session_);
-    }
+    {}
     void checkAnswer(const int expected_code, const char* name = "") {
         SCOPED_TRACE(name);
 
         parseAnswer(rcode_, result_);
         EXPECT_EQ(expected_code, rcode_) << result_->str();
     }
-    MockSession statistics_session_;
     MockXfroutClient xfrout_;
     MockSocketSessionForwarder ddns_forwarder_;
     AuthSrv server_;
@@ -106,14 +103,6 @@ TEST_F(AuthCommandTest, DISABLED_unexpectedException) {
                  runtime_error);
 }
 
-TEST_F(AuthCommandTest, sendStatistics) {
-    result_ = execAuthServerCommand(server_, "sendstats", ConstElementPtr());
-    // Just check some message has been sent.  Detailed tests specific to
-    // statistics are done in its own tests.
-    EXPECT_EQ("Stats", statistics_session_.getMessageDest());
-    checkAnswer(0);
-}
-
 void
 AuthCommandTest::stopServer() {
     result_ = execAuthServerCommand(server_, "shutdown", param_);
@@ -425,4 +414,13 @@ TEST_F(AuthCommandTest, loadZoneInvalidParams) {
                                     Element::fromJSON("{\"origin\": 10}"));
     checkAnswer(1, "Integral name");
 }
+
+TEST_F(AuthCommandTest, getStats) {
+    result_ = execAuthServerCommand(server_, "getstats",
+                                    ConstElementPtr());
+    parseAnswer(rcode_, result_);
+    // Just check some message has been sent.  Detailed tests specific to
+    // statistics are done in its own tests.
+    EXPECT_EQ(0, rcode_);
+}
 }

+ 0 - 51
src/bin/auth/tests/config_unittest.cc

@@ -79,20 +79,6 @@ TEST_F(AuthConfigTest, versionConfig) {
                         Element::fromJSON("{\"version\": 0}")));
 }
 
-TEST_F(AuthConfigTest, exceptionGuarantee) {
-    server.setStatisticsTimerInterval(1234);
-    EXPECT_EQ(1234, server.getStatisticsTimerInterval());
-    // This configuration contains an invalid item, which will trigger
-    // an exception.
-    EXPECT_THROW(configureAuthServer(
-                     server,
-                     Element::fromJSON(
-                         "{ \"no_such_config_var\": 1}")),
-                 AuthConfigError);
-    // The server state shouldn't change
-    EXPECT_EQ(1234, server.getStatisticsTimerInterval());
-}
-
 TEST_F(AuthConfigTest, badConfig) {
     // These should normally not happen, but should be handled to avoid
     // an unexpected crash due to a bug of the caller.
@@ -142,41 +128,4 @@ protected:
     AuthConfigParser* parser;
 };
 
-TEST_F(StatisticsIntervalConfigTest, setInterval) {
-    // initially the timer is not configured.
-    EXPECT_EQ(0, server.getStatisticsTimerInterval());
-
-    // initialize the timer
-    parser->build(Element::fromJSON("5"));
-    parser->commit();
-    EXPECT_EQ(5, server.getStatisticsTimerInterval());
-
-    // reset the timer with a new interval
-    delete parser;
-    parser = createAuthConfigParser(server, "statistics-interval");
-    ASSERT_NE(static_cast<void*>(NULL), parser);
-    parser->build(Element::fromJSON("10"));
-    parser->commit();
-    EXPECT_EQ(10, server.getStatisticsTimerInterval());
-
-    // disable the timer again
-    delete parser;
-    parser = createAuthConfigParser(server, "statistics-interval");
-    ASSERT_NE(static_cast<void*>(NULL), parser);
-    parser->build(Element::fromJSON("0"));
-    parser->commit();
-    EXPECT_EQ(0, server.getStatisticsTimerInterval());
-}
-
-TEST_F(StatisticsIntervalConfigTest, badInterval) {
-    EXPECT_THROW(parser->build(Element::fromJSON("\"should be integer\"")),
-                 isc::data::TypeError);
-    EXPECT_THROW(parser->build(Element::fromJSON("2.5")),
-                 isc::data::TypeError);
-    EXPECT_THROW(parser->build(Element::fromJSON("-1")), AuthConfigError);
-    // bounds check: interval value must be equal to or shorter than
-    // 86400 seconds (1 day)
-    EXPECT_NO_THROW(parser->build(Element::fromJSON("86400")));
-    EXPECT_THROW(parser->build(Element::fromJSON("86401")), AuthConfigError);
-}
 }

+ 21 - 74
src/bin/auth/tests/statistics_unittest.cc

@@ -30,7 +30,6 @@
 
 #include <dns/tests/unittest_util.h>
 
-using isc::UnitTestUtil;
 using namespace std;
 using namespace isc::cc;
 using namespace isc::dns;
@@ -77,11 +76,9 @@ private:
 
 protected:
     AuthCountersTest() : counters() {
-        counters.setStatisticsSession(&statistics_session_);
     }
     ~AuthCountersTest() {
     }
-    MockSession statistics_session_;
     AuthCounters counters;
     // no need to be inherited from the original class here.
     class MockModuleSpec {
@@ -195,25 +192,6 @@ TEST_F(AuthCountersTest, incrementRcodeCounter) {
     }
 }
 
-TEST_F(AuthCountersTest, submitStatisticsWithoutSession) {
-    // Set statistics_session to NULL and call submitStatistics().
-    // Expect to return false.
-    counters.setStatisticsSession(NULL);
-    EXPECT_FALSE(counters.submitStatistics());
-}
-
-TEST_F(AuthCountersTest, submitStatisticsWithException) {
-    // Exception SessionError and SessionTimeout will be thrown
-    // while sending statistics data.
-    // Both expect to return false.
-    statistics_session_.setThrowSessionError(true);
-    EXPECT_FALSE(counters.submitStatistics());
-    statistics_session_.setThrowSessionError(false);
-    statistics_session_.setThrowSessionTimeout(true);
-    EXPECT_FALSE(counters.submitStatistics());
-    statistics_session_.setThrowSessionTimeout(false);
-}
-
 void
 opcodeDataCheck(ConstElementPtr data, const int expected[16]) {
     const char* item_names[] = {
@@ -258,10 +236,9 @@ rcodeDataCheck(ConstElementPtr data, const int expected[17]) {
     ASSERT_EQ(static_cast<const char*>(NULL), item_names[i]);
 }
 
-
-TEST_F(AuthCountersTest, submitStatisticsWithoutValidator) {
-    // Submit statistics data.
-    // Validate if it submits correct data.
+TEST_F(AuthCountersTest, getStatisticsWithoutValidator) {
+    // Get statistics data.
+    // Validate if it answers correct data.
 
     // Counters should be initialized to 0.
     EXPECT_EQ(0, counters.getCounter(AuthCounters::SERVER_UDP_QUERY));
@@ -272,20 +249,8 @@ TEST_F(AuthCountersTest, submitStatisticsWithoutValidator) {
     counters.inc(AuthCounters::SERVER_UDP_QUERY);
     // TCP query counter is set to 1.
     counters.inc(AuthCounters::SERVER_TCP_QUERY);
-    counters.submitStatistics();
-
-    // Destination is "Stats".
-    EXPECT_EQ("Stats", statistics_session_.msg_destination);
-    // Command is "set".
-    EXPECT_EQ("set", statistics_session_.sent_msg->get("command")
-                         ->get(0)->stringValue());
-    EXPECT_EQ("Auth", statistics_session_.sent_msg->get("command")
-                         ->get(1)->get("owner")->stringValue());
-    EXPECT_EQ(statistics_session_.sent_msg->get("command")
-              ->get(1)->get("pid")->intValue(), getpid());
-    ConstElementPtr statistics_data = statistics_session_.sent_msg
-                                          ->get("command")->get(1)
-                                          ->get("data");
+    ConstElementPtr statistics_data = counters.getStatistics();
+
     // 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());
@@ -318,61 +283,53 @@ updateRcodeCounters(AuthCounters &counters, const int expected[17]) {
     }
 }
 
-TEST_F(AuthCountersTest, submitStatisticsWithOpcodeCounters) {
+TEST_F(AuthCountersTest, getStatisticsWithOpcodeCounters) {
     // 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");
+    ConstElementPtr statistics_data = counters.getStatistics();
     opcodeDataCheck(statistics_data, opcode_results);
 }
 
-TEST_F(AuthCountersTest, submitStatisticsWithAllOpcodeCounters) {
+TEST_F(AuthCountersTest, getStatisticsWithAllOpcodeCounters) {
     // 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");
+    ConstElementPtr statistics_data = counters.getStatistics();
     opcodeDataCheck(statistics_data, opcode_results);
 }
 
-TEST_F(AuthCountersTest, submitStatisticsWithRcodeCounters) {
+TEST_F(AuthCountersTest, getStatisticsWithRcodeCounters) {
     // Increment some of the rcode counters.  Then they should appear in the
     // submitted data; others shouldn't
     const int rcode_results[17] = { 1, 2, 3, 4, 5, 6, 7, 8, 9,
                                     10, 0, 0, 0, 0, 0, 0, 11 };
     updateRcodeCounters(counters, rcode_results);
-    counters.submitStatistics();
-    ConstElementPtr statistics_data = statistics_session_.sent_msg
-        ->get("command")->get(1)->get("data");
+    ConstElementPtr statistics_data = counters.getStatistics();
     rcodeDataCheck(statistics_data, rcode_results);
 }
 
-TEST_F(AuthCountersTest, submitStatisticsWithAllRcodeCounters) {
+TEST_F(AuthCountersTest, getStatisticsWithAllRcodeCounters) {
     // Increment all rcode counters.  Then they should appear in the
     // submitted data.
     const int rcode_results[17] = { 1, 1, 1, 1, 1, 1, 1, 1, 1,
                                      1, 1, 1, 1, 1, 1, 1, 1 };
     updateOpcodeCounters(counters, rcode_results);
-    counters.submitStatistics();
-    ConstElementPtr statistics_data = statistics_session_.sent_msg
-        ->get("command")->get(1)->get("data");
+    ConstElementPtr statistics_data = counters.getStatistics();
     opcodeDataCheck(statistics_data, rcode_results);
 }
 
-TEST_F(AuthCountersTest, submitStatisticsWithValidator) {
+TEST_F(AuthCountersTest, getStatisticsWithValidator) {
 
     //a validator for the unittest
     AuthCounters::validator_type validator;
     ConstElementPtr el;
 
-    // Submit statistics data with correct statistics validator.
+    // Get statistics data with correct statistics validator.
     validator = boost::bind(
         &AuthCountersTest::MockModuleSpec::validateStatistics,
         &module_spec_, _1, true);
@@ -392,24 +349,14 @@ TEST_F(AuthCountersTest, submitStatisticsWithValidator) {
     // TCP query counter is set to 1.
     counters.inc(AuthCounters::SERVER_TCP_QUERY);
 
-    // checks the value returned by submitStatistics
-    EXPECT_TRUE(counters.submitStatistics());
-
-    // Destination is "Stats".
-    EXPECT_EQ("Stats", statistics_session_.msg_destination);
-    // Command is "set".
-    EXPECT_EQ("set", statistics_session_.sent_msg->get("command")
-                         ->get(0)->stringValue());
-    EXPECT_EQ("Auth", statistics_session_.sent_msg->get("command")
-                         ->get(1)->get("owner")->stringValue());
-    ConstElementPtr statistics_data = statistics_session_.sent_msg
-                                          ->get("command")->get(1)
-                                          ->get("data");
+    // checks the value returned by getStatistics
+    ConstElementPtr statistics_data = counters.getStatistics();
+
     // 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());
 
-    // Submit statistics data with incorrect statistics validator.
+    // Get statistics data with incorrect statistics validator.
     validator = boost::bind(
         &AuthCountersTest::MockModuleSpec::validateStatistics,
         &module_spec_, _1, false);
@@ -418,7 +365,7 @@ TEST_F(AuthCountersTest, submitStatisticsWithValidator) {
 
     counters.registerStatisticsValidator(validator);
 
-    // checks the value returned by submitStatistics
-    EXPECT_FALSE(counters.submitStatistics());
+    // checks the value returned by getStatistics
+    EXPECT_FALSE(counters.getStatistics());
 }
 }