Opened 8 years ago

Closed 7 years ago

#1745 closed defect (fixed)

adding pid to auth (and resolver) log ID

Reported by: jinmei Owned by: muks
Priority: medium Milestone: Sprint-20121204
Component: logging Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 2 Add Hours to Ticket: 0
Total Hours: 0.55 Internal?: no

Description

An accidental commit reminded me of this: we'll need some way to
distinguish log messages from multiple instances of b10-auth/resolver.
While I debugged troubles with multiple instances (which eventually
turned out to be ASIO bugs), I did this quick hack and found it quite
useful:

--- a/src/bin/auth/main.cc
+++ b/src/bin/auth/main.cc
@@ -25,6 +25,8 @@
 #include <cassert>
 #include <iostream>
 
+#include <boost/lexical_cast.hpp>
+
 #include <exceptions/exceptions.h>
 
 #include <util/buffer.h>
@@ -116,7 +118,8 @@ main(int argc, char* argv[]) {
     }
 
     // Initialize logging.  If verbose, we'll use maximum verbosity.
-    isc::log::initLogger(AUTH_NAME,
+    isc::log::initLogger(string(AUTH_NAME) + " (" +
+                         boost::lexical_cast<string>(getpid()) + ")",
                          (verbose ? isc::log::DEBUG : isc::log::INFO),
                          isc::log::MAX_DEBUG_LEVEL, NULL);

Although it wouldn't be ideal, I'd suggest doing it for both auth and
resolver for short-to-middle term workaround. I believe it's much
better than producing the non-distinguishable logs.

This should be a very easy task, btw (and I think it's a kind of "bug
fix" as the current situation is really inconvenient).

Subtickets

Change History (12)

comment:1 Changed 8 years ago by jinmei

If we do this:
http://bind10.isc.org/ticket/1751#comment:2
it's probably better to use that instance identifier for this purpose.

But if we don't plan to do it soon, I'd suggest introducing the
workaround of this ticket in the meantime.

comment:2 Changed 8 years ago by stephen

This might interact badly with the wildcard specification of logger components (e.g. at the moment you can add a specification for "b10-auth.*" - "b10-auth*" does not match).

What might be better is to make the PID a separate component of the logger name, e.g.

isc::log::initLogger(string(AUTH_NAME) + "."
    + boost::lexical_cast<string>(getpid()));

... so that messages come from loggers with names like "b10-auth.42.cc" and components can be specified as "b10-auth.*.*"

comment:3 Changed 8 years ago by jreed

What about using some process count instead of PID? So if have four auth components, then have b10-auth.1.cc to b10-auth.4.cc.

comment:4 Changed 8 years ago by jelte

  • Estimated Difficulty changed from 0 to 2

comment:5 Changed 7 years ago by jinmei

  • Milestone set to Next-Sprint-Proposed

comment:6 Changed 7 years ago by shane

  • Milestone set to Next-Sprint-Proposed

We could have the logger always include process ID, since the process itself doesn't know if there will be multiple copies running.

comment:7 Changed 7 years ago by shane

See the description of #2177 for another view.

comment:8 Changed 7 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20121204

comment:9 Changed 7 years ago by muks

  • Owner set to muks
  • Status changed from new to assigned

Picking

comment:10 Changed 7 years ago by muks

  • Owner changed from muks to UnAssigned
  • Status changed from assigned to reviewing

Up for review. Instead of adding the pid to the logger name, the log4cplus pattern was updated to include the pid. Here is a snippet of the output generated by features/multi_instance.feature:

2012-11-26 03:25:01.289 DEBUG [b10-auth.cc/12036] CC_ESTABLISHED successfully connected to message queue daemon
2012-11-26 03:25:01.290 DEBUG [b10-auth.cc/12036] CC_SUBSCRIBE subscribing to communication group Auth
2012-11-26 03:25:01.291 DEBUG [b10-auth.cc/12037] CC_ESTABLISH trying to establish connection with message queue daemon at /home/muks/bind10/msgq_socket
2012-11-26 03:25:01.291 DEBUG [b10-auth.cc/12037] CC_ESTABLISHED successfully connected to message queue daemon

comment:11 Changed 7 years ago by vorner

  • Owner changed from UnAssigned to muks
  • Total Hours changed from 0 to 0.55

Hello

It looks good for merge.

comment:12 Changed 7 years ago by muks

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged to master branch in commit fc8bbf3d438e8154e7c2bdd322145a7f7854dc6a:

* 4140b1a [1745] Add pid to all log messages

Also added the following ChangeLog entry:

+509.   [func]          muks
+       Log messages now include the pid of the process that logged the
+       message.
+       (Trac #1745, git fc8bbf3d438e8154e7c2bdd322145a7f7854dc6a)
+

Resolving as fixed. Thank you for the review Michal.

Note: See TracTickets for help on using tickets.