[Xorp-hackers] PATCH: Logging improvements, fix artificial deal for xorpsh commit.

J.T. Conklin jtc at acorntoolworks.com
Fri Oct 9 07:40:50 PDT 2009


Bruce Simpson <bms at incunabulum.net> writes:
> Comments:
>    * It should be possible to turn off the millisecond logging if 
> desired. Whilst it's certainly a useful feature to have when debugging 
> time contingent code, it does add clutter to the output.

I think that now systems are so fast that sub-second timestamps should
be almost always be used for log/event timestamps.  This is especially
true in distributed systems where log messages from separate programs
are/need to be merged into one stream. Without sub-second timestamps,
everything appears to happen at one time.

For what it's worth, the new RFC5424 says the "originator SHOULD
include TIME-SECFRAC if its clock accuracy and performance permit".
While we don't currently format log messages according to the RFC, we
probably open a Trac ticket along those lines.  Emitting RFC compliant
log messages will make it easier for automated log analysis and data-
base systems to handle XORP output.

>     * Perhaps putting it under the other debug knobs in SConstruct would 
> be a good idea?

As for configure knobs to control this (and other) log behavior...  I
think it's worth considering totally rototilling XORP's log subsystem
and using log4cxx (or log4j/log4cxx inspired code of our own).  If we
took full advantage of the framework, we could have much more finer
grain control of log messages by defining logger hierarchies (eg., we
could enable messages just from the xorp.bgp.foo.bar logger).  We could 
also define/select format specifications with a config file, avoiding 
compiling in behavior like whether sub-second timestamps would be used.

It's a big project, but I think has the potential of similarly big
rewards.

In the short term, I think we should change the log output to be RFC
5424 compliant, including sub-second timestamps.

>    * %llu is not a portable format specifier, and 'unsigned long long' 
> is not a portable type, please don't use them in portable code.

As Ben found discovered, your suggestion to cast to intmax_t and use
the %j format specifier doesn't work on the older systems.

I think fixed sized integral types like int64_t, uint64_t, etc. and
the corresponding macros like PRId64, PRIu64, etc.  were interduced in
C90, and should be the most portable.  And, if we run into any systems
that don't have them, it should be easy enough to define the types and
macros in xorp_config.h.

    --jtc

-- 
J.T. Conklin



More information about the Xorp-hackers mailing list