Opened 9 years ago
Closed 8 years ago
#738 closed enhancement (complete)
Conversion of b10-auth to use new logging interface
Reported by: | stephen | Owned by: | stephen |
---|---|---|---|
Priority: | medium | Milestone: | Sprint-20110628 |
Component: | b10-auth | Version: | |
Keywords: | Cc: | ||
CVSS Scoring: | Parent Tickets: | ||
Sensitive: | no | Defect Severity: | N/A |
Sub-Project: | DNS | Feature Depending on Ticket: | |
Estimated Difficulty: | 6.0 | Add Hours to Ticket: | 0 |
Total Hours: | 0 | Internal?: | no |
Description
Examining the b10-auth code and changing any logging calls to use the new logging interface.
This involves:
- Extracting messages into a separate message file. (This may be a unique file for b10-auth or it may be a file shared between separate programs).
- Using the new logging calls.
Subtickets
Change History (10)
comment:1 Changed 9 years ago by shane
- Estimated Difficulty changed from 0.0 to 6
- Milestone changed from Year 3 Task Backlog to Sprint-20110419
- Priority changed from major to minor
comment:2 Changed 9 years ago by stephen
- Milestone changed from Sprint-20110503 to Year 3 Task Backlog
comment:3 Changed 8 years ago by shane
- Defect Severity set to N/A
- Milestone changed from Year 3 Task Backlog to Sprint-20110628
- Priority changed from minor to major
- Sub-Project set to DNS
comment:4 Changed 8 years ago by stephen
- Owner set to stephen
- Status changed from new to assigned
comment:5 Changed 8 years ago by stephen
- Owner changed from stephen to UnAssigned
- Status changed from assigned to reviewing
comment:6 Changed 8 years ago by vorner
- Owner changed from UnAssigned to vorner
comment:7 Changed 8 years ago by vorner
- Owner changed from vorner to stephen
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?
} 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.
// TODO: SHould this be LOG_ERROR? LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_COMMAND_FAILED) .arg(command_id).arg(ex.what());
With regards
comment:8 Changed 8 years ago by stephen
- Owner changed from stephen to vorner
First of, the distcheck fails with this...
This turned out to be auth_log.h containing the line '#include "auth_messages.h"'. auth_messages.h is created in the build directory, and so wasn't being found when the source and build directories were different. Changing the file specification to '<auth/auth_messages.h>' fixed the problem.
Then, I'm thinking, the arg(exc.what()) is quite common. Would it help if we defined template specialization for exception?
I think it would. We talked about logging exceptions some time ago (should an exception be automatically logged when raised?) and it might be worth discussing the whole issue on bind10-dev.
And, finally, few minor points:...
Typos and indentation fixed, some messages re-worded.
I think this should be LOG_ERROR, after all, failing command should be rare ...
Agreed. It was only LOG_DEBUG because in the original code, the message was only being issued if verbose_mode was set.
comment:9 Changed 8 years ago by vorner
- Owner changed from vorner to stephen
Hello
It looks good, please merge.
Thanks
comment:10 Changed 8 years ago by stephen
- Resolution set to complete
- Status changed from reviewing to closed
Merged as commit c021505a1a0d6ecb15a8fd1592b94baff6d115f4
Messages output to stderr have been substituted by the new-style logging.
Also changes was the removal of the "verbose_mode" flag - this is not needed with the logging code.