Log: move global logger storage to logger object + enable log-to-file in daemon mode

[anonimal]

By submitting this issue, I confirm the following:

• I have read and understood the contributor guide.
• I have checked that the issue I am reporting can be replicated or that the feature I am suggesting is not present.
• I have checked opened or recently closed pull requests for existing solutions/implementations to my issue/suggestion.

Place an X inside the bracket to confirm

• I confirm.

---

As noted in c8870c0

// TODO(anonimal):
// Boost.Log uses an "application-wide singleton" (note: our logger/sink setup applies globally from instance configuration)
// As a result, logging will not work when in daemon mode. http://www.boost.org/doc/libs/1_63_0/libs/log/doc/html/log/rationale/fork_support.html
// We've worked around this problem in the past by using some very gross hacking but we may be able to apply a cleaner work-around so we can set this up entirely in the core namespace
// (we could create an (inheritable?) logging class with overloaded stream operator and adjust our logging initialization and macro accordingly, or consider other options)
// Also note that a singleton will effect having multiple logging library options (there's no need to do that though when we have huge flexibility with sinks)

While we here (unrelated), we should add tags and a global exception handler (if this is needed sooner then we should open a new feature request).

[coneiric]

(we could create an (inheritable?) logging class with overloaded stream operator and adjust our logging initialization and macro accordingly, or consider other options)

One idea I had along these lines was to create a base logging class, like you suggest, and inherit+instantiate in each module namespace.

So each nampespace's macro wouldn't need to change when calling it, and would get initialized with that module's logger.

How I'm thinking of it so far:

In core/util/base_logger.{h,cc}:

all the current definitions in core/util/log.{h,cc} without the macros

Then in core/util/log.{h,cc}:

namespace kovri {
namespace core {

class CoreLogger : public BaseLogger {...};
CoreLogger co_Logger;
#define LOG(severity)  BOOST_LOG_SEV(co_Logger, boost::log::trivial::severity)

Would take a similar approach for client, app, and util. Does this design make sense, and/or follow your suggestions?

Also, thought about giving each class it's own logger, but that seemed to get out of control quickly (already over 64 classes that use logging).

[anonimal]

Also, thought about giving each class it's own logger, but that seemed to get out of control quickly (already over 64 classes that use logging).

Yes, don't do that.

One idea I had along these lines was to create a base logging class, like you suggest, and inherit+instantiate in each module namespace.
So each nampespace's macro wouldn't need to change when calling it, and would get initialized with that module's logger.

I'm not sure what you mean because I don't think that's the right language for what you're trying to describe. Can you rephrase?

[coneiric]

Can you rephrase?

Yeah, I guess that was a bit confusing... I meant to remove the global logger, and setup a logger inside each namespace.

So, each namespace would have its own logger macro that uses a logger class specific to that namespace. Each logger class could inherit from a base logger class defined in the core (or other) namespace.

What I meant by not needing to change the call, is that LOG(severity) << "some log message" wouldn't need to change. Since the macro for each namespace would be initialized with that module's logger class, instead of the global logger.

[anonimal]

I don't see the need to have 3 loggers according to namespace since they all can pull from the core library. Why do you want to do this?

[coneiric]

It was just the middle-ground between logger-for-every-class and global logger.

Really, I don't have strong opinions on 3 loggers, just throwing out halfway-plausible ideas.

Instead, are you saying core would expose its logger through an API to client and util (whatever needs it)?

[anonimal]

How else do you propose to initialize with the appropriate class name? Mind you, this logger object can also include an exception object - which would also be appropriately initialized every time the logger class is inherited.

[coneiric]

How else do you propose to initialize with the appropriate class name?

No proposals :) This sounds good to me.

this logger object can also include an exception object

Very cool, I didn't realize that. This definitely seems like the way to go. So, each class that needs logging would inherit the core logger class, which includes a version of what kovri::core::Exception does now? (just making sure I understand)

[anonimal]

So, each class that needs logging would inherit the core logger class, which includes a version of what kovri::core::Exception does now?

Yes, that's one approach.

[Wuodan]

When you improve --daemon logging: It would be nice to append to the days log file instead of overwriting it on kovri start. Just an idea.

[anonimal]

It would be nice to append to the days log file instead of overwriting it on kovri start.

How about appending a timestamp to the filename for start-of-logging? That way you don't have to sift through a day's worth of log to see when you last started the router.

[anonimal]

NOTICE: THIS ISSUE HAS BEEN MOVED TO GitLab. Please continue the discussion there. See #1013 for details.