[3806] Address second round of review comments.

1) Removed the srv-hooks logger
2) Updated DHCP4_NCR_CREATE message description
3) Updated logging doc.

doc/guide/logging.xml

@@ -180,12 +180,10 @@
           </listitem>
           <listitem>
             <simpara><command>kea-dhcp4.hooks</command> - this logger is used
-            by the libkea-hooks library to log messages related to the execution
-            of callouts for various hook points. This logger doesn't log messages
-            pertaining to actions on the server, being the result of the
-            callout execution, e.g. skipping packet parsing by the server
-            because the callout has indicated that it has parsed the packet
-            already.</simpara>
+            to log messages pertaining to management of hooks libraries, e.g.
+            registration and deregistration of the libraries, and to the
+            initialization of callouts execution for various hook points within
+            the DHCPv4 server.</simpara>
           </listitem>
           <listitem>
             <simpara><command>kea-dhcp4.ddns</command> - this logger is
@@ -226,15 +224,6 @@
             </simpara>
           </listitem>
           <listitem>
-            <simpara><command>kea-dhcp4.server-hooks</command> - this logger is
-            used by the DHCPv4 server to log messages pertaining to the
-            preparations for the callouts execution by the server and to the
-            actions taken by the server as a result of the callouts execution.
-            Note that it is a different logger than the
-            <command>kea-dhcp4.hooks</command> logger.
-          </simpara>
-          </listitem>
-          <listitem>
             <simpara><command>kea-dhcp6</command> - this is the root logger for
             the DHCPv6 server. All components used by the DHCPv6 server inherit
             the settings from this logger if there is no specialized logger
@@ -253,12 +242,8 @@
           </listitem>
           <listitem>
             <simpara><command>kea-dhcp6.hooks</command> - this logger is used
-            by the hooks framework to log messages related to the execution of
-            callouts for various hook points. This logger doesn't log messages
-            which pertain to actions on the server side being the result of
-            the callout execution, e.g. skipping packet parsing by the server
-            because the callout has indicated that it has parsed the packet
-            already.</simpara>
+            to log messages pertaining to management of hooks libraries, e.g.
+            registration and deregistration of the libraries.</simpara>
           </listitem>
           <listitem>
             <simpara><command>kea-dhcp6.hosts</command> - this logger is used

src/bin/dhcp4/dhcp4_log.cc

@@ -27,7 +27,6 @@ const char* DHCP4_PACKET_LOGGER_NAME = "packets";
 const char* DHCP4_OPTIONS_LOGGER_NAME = "options";
 const char* DHCP4_DDNS_LOGGER_NAME = "ddns";
 const char* DHCP4_LEASE_LOGGER_NAME = "leases";
-const char* DHCP4_SRV_HOOKS_LOGGER_NAME = "server-hooks";
 
 isc::log::Logger dhcp4_logger(DHCP4_APP_LOGGER_NAME);
 isc::log::Logger bad_packet_logger(DHCP4_BAD_PACKET_LOGGER_NAME);
@@ -35,7 +34,6 @@ isc::log::Logger packet_logger(DHCP4_PACKET_LOGGER_NAME);
 isc::log::Logger options_logger(DHCP4_OPTIONS_LOGGER_NAME);
 isc::log::Logger ddns_logger(DHCP4_DDNS_LOGGER_NAME);
 isc::log::Logger lease_logger(DHCP4_LEASE_LOGGER_NAME);
-isc::log::Logger srv_hooks_logger(DHCP4_SRV_HOOKS_LOGGER_NAME);
 
 } // namespace dhcp
 } // namespace isc

src/bin/dhcp4/dhcp4_log.h

@@ -89,9 +89,6 @@ extern const char* DHCP4_DDNS_LOGGER_NAME;
 /// @brief Name of the logger for lease allocation logic.
 extern const char* DHCP4_LEASE_LOGGER_NAME;
 
-/// @brief Name of the logger for hooks execution by the server.
-extern const char* DHCP4_SRV_HOOKS_LOGGER_NAME;
-
 //@}
 
 /// @name Loggers used by the DHCPv4 server
@@ -130,12 +127,6 @@ extern isc::log::Logger ddns_logger;
 /// This logger is used to issue log messages related to lease allocation.
 extern isc::log::Logger lease_logger;
 
-/// @brief Logger for hooks execution by the server.
-///
-/// This logger is used to issue log messages related to use of hooks in
-/// the DHCPv4 server.
-extern isc::log::Logger srv_hooks_logger;
-
 //@}
 
 } // namespace dhcp4

src/bin/dhcp4/dhcp4_messages.mes

@@ -277,8 +277,9 @@ The third argument contains the error details.
 This debug message message is issued when the server is starting to send
 name change requests to the D2 module to update records for the client
 in the DNS. This includes removal of old records and addition of the
-new records as required. The argument includes the client and the
-transaction identification information.
+new records as required. Details of the name change requests will be
+logged in additional log entries. The argument includes the client
+and the transaction identification information.
 
 % DHCP4_NCR_CREATION_FAILED %1: failed to generate name change requests for DNS: %2
 This message indicates that server was unable to generate NameChangeRequests

src/bin/dhcp4/dhcp4_srv.cc

@@ -38,6 +38,7 @@
 #include <dhcpsrv/utils.h>
 #include <dhcpsrv/utils.h>
 #include <hooks/callout_handle.h>
+#include <hooks/hooks_log.h>
 #include <hooks/hooks_manager.h>
 #include <util/strutil.h>
 
@@ -278,7 +279,7 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& query) const {
         // will be selected. Packet processing will continue, but it will
         // be severely limited (i.e. only global options will be assigned)
         if (callout_handle->getSkip()) {
-            LOG_DEBUG(srv_hooks_logger, DBG_DHCP4_HOOKS,
+            LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                       DHCP4_HOOK_SUBNET4_SELECT_SKIP)
                 .arg(query->getLabel());
             return (Subnet4Ptr());
@@ -430,7 +431,7 @@ Dhcpv4Srv::run() {
             // stage means that callouts did the parsing already, so server
             // should skip parsing.
             if (callout_handle->getSkip()) {
-                LOG_DEBUG(srv_hooks_logger, DBG_DHCP4_DETAIL, DHCP4_HOOK_BUFFER_RCVD_SKIP)
+                LOG_DEBUG(hooks_logger, DBG_DHCP4_DETAIL, DHCP4_HOOK_BUFFER_RCVD_SKIP)
                     .arg(query->getRemoteAddr().toText())
                     .arg(query->getLocalAddr().toText())
                     .arg(query->getIface());
@@ -504,7 +505,7 @@ Dhcpv4Srv::run() {
             // processing step would to process the packet, so skip at this
             // stage means drop.
             if (callout_handle->getSkip()) {
-                LOG_DEBUG(srv_hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_RCVD_SKIP)
+                LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_RCVD_SKIP)
                     .arg(query->getLabel());
                 continue;
             }
@@ -586,7 +587,7 @@ Dhcpv4Srv::run() {
             // processing step would to send the packet, so skip at this
             // stage means "drop response".
             if (callout_handle->getSkip()) {
-                LOG_DEBUG(srv_hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_SEND_SKIP)
+                LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_SEND_SKIP)
                     .arg(query->getLabel());
                 skip_pack = true;
             }
@@ -626,7 +627,7 @@ Dhcpv4Srv::run() {
                 // processing step would to parse the packet, so skip at this
                 // stage means drop.
                 if (callout_handle->getSkip()) {
-                    LOG_DEBUG(srv_hooks_logger, DBG_DHCP4_HOOKS,
+                    LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                               DHCP4_HOOK_BUFFER_SEND_SKIP)
                         .arg(rsp->getLabel());
                     continue;
@@ -1722,7 +1723,7 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
             // stage means "drop response".
             if (callout_handle->getSkip()) {
                 skip = true;
-                LOG_DEBUG(srv_hooks_logger, DBG_DHCP4_HOOKS,
+                LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                           DHCP4_HOOK_LEASE4_RELEASE_SKIP)
                     .arg(release->getLabel());
             }