BIND 10 #738: Conversion of b10-auth to use new logging interface

BIND 10 Development do-not-reply at isc.org
Wed Jun 22 12:06:45 UTC 2011


#738: Conversion of b10-auth to use new logging interface
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  stephen
  stephen                            |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20110628
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  b10-auth                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  6.0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => stephen


Comment:

 Hello

 First of, the distcheck fails with this:
 {{{
 g++ -DHAVE_CONFIG_H -I. -I../../../../../src/bin/auth/benchmarks
 -I../../../..  -I../../../../../src/lib -I../../../../src/lib
 -I../../../../../src/bin -I../../../../src/bin  -I../../../../../ext/asio
 -I../../../../../ext/coroutine -DASIO_DISABLE_THREADS=1 -Wall -Wextra
 -Wwrite-strings -Woverloaded-virtual -Wno-sign-compare -Werror -fPIC -g
 -O2 -MT auth_log.o -MD -MP -MF .deps/auth_log.Tpo -c -o auth_log.o `test
 -f '../auth_log.cc' || echo
 '../../../../../src/bin/auth/benchmarks/'`../auth_log.cc
 In file included from
 ../../../../../src/bin/auth/benchmarks/../auth_log.cc:17:0:
 ../../../../../src/bin/auth/benchmarks/../auth_log.h:19:27: fatal error:
 auth_messages.h: No such file or directory
 compilation terminated.
 make[7]: *** [auth_log.o] Error 1
 make[7]: *** Waiting for unfinished jobs....
 In file included from
 ../../../../../src/bin/auth/benchmarks/../statistics.cc:16:0:
 ../../../../../src/bin/auth/auth_log.h:19:27: fatal error:
 auth_messages.h: No such file or directory
 compilation terminated.
 make[7]: *** [statistics.o] Error 1
 In file included from
 ../../../../../src/bin/auth/benchmarks/../auth_srv.cc:62:0:
 ../../../../../src/bin/auth/auth_log.h:19:27: fatal error:
 auth_messages.h: No such file or directory
 compilation terminated.
 make[7]: *** [auth_srv.o] Error 1
 mv -f .deps/query_bench.Tpo .deps/query_bench.Po
 mv -f .deps/auth_config.Tpo .deps/auth_config.Po
 make[7]: Leaving directory
 `/home/vorner/work/bind10/bind10-devel-20110519/_build/src/bin/auth/benchmarks'
 make[6]: *** [all-recursive] Error 1
 make[6]: Leaving directory
 `/home/vorner/work/bind10/bind10-devel-20110519/_build/src/bin/auth'
 make[5]: *** [all] Error 2
 make[5]: Leaving directory
 `/home/vorner/work/bind10/bind10-devel-20110519/_build/src/bin/auth'
 make[4]: *** [all-recursive] Error 1
 make[4]: Leaving directory
 `/home/vorner/work/bind10/bind10-devel-20110519/_build/src/bin'
 make[3]: *** [all-recursive] Error 1
 make[3]: Leaving directory
 `/home/vorner/work/bind10/bind10-devel-20110519/_build/src'
 make[2]: *** [all-recursive] Error 1
 make[2]: Leaving directory
 `/home/vorner/work/bind10/bind10-devel-20110519/_build'
 make[1]: *** [all] Error 2
 make[1]: Leaving directory
 `/home/vorner/work/bind10/bind10-devel-20110519/_build'
 make: *** [distcheck] Error 1
 }}}

 I guess this is some problem with ordering.

 Then, I'm thinking, the arg(exc.what()) is quite common. Would it help if
 we defined template specialization for exception?

 And, finally, few minor points:
  * `AUTH_COMMAND_FAILED` ‒ It is not clear about which command it talks. I
 guess it's about the command channel commands, but admin oblivious of the
 architecture might think which DNS commands there are.
  * `AUTH_LOAD_TSIG` says the TSIGs are accessed, which is not exact. They
 are being requested from the configuration database.
  * `AUTH_LOAD_ZONE` ‒ should „names zone“ be „named zone“?
  * „For some reason, the authoritative server has no session with the
 statistics module is available.“ ‒ this sounds odd, aren't there two
 verbs?
  * `AUTH_RECEIVED_SENDSTATS` ‒ issues → issued?
  * Indentation?
 {{{#!C++
     } catch (const Exception& ex) {
 -        if (verbose_mode_) {
 -            cerr << "[b10-auth] failed to notify Zonemgr: " << ex.what()
 << endl;
 -        }
 +            LOG_ERROR(auth_logger, AUTH_ZONEMGR_COMMS).arg(ex.what());
          return (false);
 }}}
  * I think this should be LOG_ERROR, after all, failing command should be
 rare and better if people complain that it is too verbose than if it is
 rotten inside and nobody knows.
 {{{#!C++
  // TODO: SHould this be LOG_ERROR?
          LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_COMMAND_FAILED)
                    .arg(command_id).arg(ex.what());
 }}}

 With regards

-- 
Ticket URL: <https://bind10.isc.org/ticket/738#comment:7>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list