Project

General

Profile

Feature #3949

Use ndn-cxx logging facility

Added by Ashlesh Gawande almost 2 years ago. Updated 12 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Start date:
02/06/2017
Due date:
06/09/2017
% Done:

100%

Estimated time:

Description

If possible use logging facility provided by ndn-cxx and get rid of log4cxx dependency.

https://redmine.named-data.net/issues/3562


Related issues

Related to ndn-cxx - Feature #3562: Logging facilityClosed

Related to NLSR - Task #1795: Make NLSR a daemon process with -d optionClosed2014-07-23

Related to NLSR - Feature #2046: Add ability to specify logging modules in configuration fileAbandoned2014-10-09

Related to NLSR - Bug #2809: Check log4cxx.properties.sample.in to make sure each parameter works correctlyClosed2015-05-13

Related to mini-ndn - Task #4435: Correctly set log level and log re-direct after NLSR switches to ndn-cxx loggerClosed2018-01-05

Is duplicate of NLSR - Task #2408: Potential issue on FreeBSD platformAbandoned

History

#1 Updated by Ashlesh Gawande almost 2 years ago

  • Assignee set to Damian Coomes

#2 Updated by Junxiao Shi almost 2 years ago

  • Tracker changed from Task to Feature

Does ndn-cxx already contain some function that logs directly to file? I see two setDestination() functions in ndn-cxx/blob/master/src/util/logging.hpp, but I am not sure if either one is the appropriate method.

Yes, you may log to a file with setDestination.

// g++ -o x -std=c++11 x.cpp $(pkg-config --cflags --libs libndn-cxx) -DBOOST_LOG_DYN_LINK
#include <ndn-cxx/util/logger.hpp>
#include <ndn-cxx/util/logging.hpp>
#include <fstream>

NDN_LOG_INIT(MyLogModule);

int
main()
{
  ndn::util::Logging::setLevel("MyLogModule", ndn::util::LogLevel::WARN);
  ndn::util::Logging::setDestination(std::make_shared<std::ofstream>("/tmp/x.log"));

  NDN_LOG_FATAL("my-fatal");
  NDN_LOG_ERROR("my-error");
  NDN_LOG_WARN("my-warning");
  NDN_LOG_INFO("my-info");

  ndn::util::Logging::flush();
}

#3 Updated by Damian Coomes almost 2 years ago

Awesome! Thank you for clarifying.

#4 Updated by Ashlesh Gawande almost 2 years ago

When NLSR is daemonized we will lose these logs.
Daemonization needs to be done before logging.

#5 Updated by Ashlesh Gawande almost 2 years ago

#6 Updated by Ashlesh Gawande almost 2 years ago

  • Related to Task #1795: Make NLSR a daemon process with -d option added

#7 Updated by Damian Coomes almost 2 years ago

ndn::util::Logging::setLevel("*", ndn::util::LogLevel::DEBUG);
ndn::util::Logging::setDestination(std::make_sharedstd::ofstream("/tmp/x.log"));

Junxiao, this is the code I am using currently, and I have it in nlsr-runner.cpp. Right now I am just using * for the moduleName but what do you think is the best way to obtain the moduleName for logging?

#8 Updated by Alex Afanasyev almost 2 years ago

It really depends on what you want to log. Module name is what is defined by NDN_LOG_INIT macro in .cpp files. Whatever you specified there is your module, unless I'm misunderstanding your question.

#9 Updated by Damian Coomes almost 2 years ago

Yeah, that makes sense. I think I was having some confusion going from log4cxx to ndn-cxx logging and was trying to figure out a replacement for log4cxx's INIT_LOGGERS function located in nlsr/src/logger.hpp. So the code immediately below (located in ndn-cxx/src/util/logger.hpp) is what initializes a new logger for the given module name whenever a file in nlsr contains INIT_LOGGER(moduleName), right?

#define NDN_LOG_INIT(name) \
namespace { \
inline ::ndn::util::Logger& getNdnCxxLogger() \
{ \
static ::ndn::util::Logger logger(BOOST_STRINGIZE(name)); \
return logger; \
} \
} \

Does the code immediately below (from nlsr/src/conf-file-processor.cpp) obtain the log-level setting from nlsr.conf? If so, does that mean that with both the source code blocks presented, there is no longer a need for an INIT_LOGGERS function?

std::string logLevel = section.get("log-level", "INFO");

if (isValidLogLevel(logLevel)) {
m_nlsr.getConfParameter().setLogLevel(logLevel);
}
else {
std::cerr << "Invalid value for log-level ";
std::cerr << "Valid values: ALL, TRACE, DEBUG, INFO, WARN, ERROR, NONE" << std::endl;
return false;
}

#10 Updated by Damian Coomes almost 2 years ago

  • Status changed from New to In Progress
  • % Done changed from 0 to 50

#11 Updated by Damian Coomes almost 2 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 50 to 90

#12 Updated by Nicholas Gordon over 1 year ago

  • Related to Task #2408: Potential issue on FreeBSD platform added

#13 Updated by Nicholas Gordon over 1 year ago

  • Related to Feature #2046: Add ability to specify logging modules in configuration file added

#14 Updated by Nicholas Gordon over 1 year ago

  • Related to Bug #2809: Check log4cxx.properties.sample.in to make sure each parameter works correctly added

#15 Updated by Nicholas Gordon over 1 year ago

  • Due date set to 06/09/2017
  • Target version set to v0.4.0

#16 Updated by Nicholas Gordon over 1 year ago

  • Related to deleted (Task #2408: Potential issue on FreeBSD platform)

#17 Updated by Nicholas Gordon over 1 year ago

  • Is duplicate of Task #2408: Potential issue on FreeBSD platform added

#18 Updated by Nicholas Gordon about 1 year ago

  • Target version changed from v0.4.0 to v0.5.0

#19 Updated by Ashlesh Gawande about 1 year ago

With log4cxx, John configures the logging to be a rolling log with 1 MB max file size and to keep up to 10 files.
So we need to preserve this behavior with ndn-cxx logger. Boost log supports rolling file but seems like there is no wrapper in ndn-cxx to do this yet.

#20 Updated by Ashlesh Gawande about 1 year ago

After discussion in today's NFD call:
It is okay to just have NLSR logs directed to std::clog
Log rotation should be handled by the system (logrotated, journald)

I guess we can keep directing to a file as suggested in https://redmine.named-data.net/issues/3949#note-2
and have the system handle the rotation.
This way user don't have to manually redirect to a file and we don't have to change Mini-NDN to redirect the logs when we start NLSR.

#21 Updated by Junxiao Shi about 1 year ago

I guess we can keep directing to a file as suggested in https://redmine.named-data.net/issues/3949#note-2
and have the system handle the rotation.

This won’t work because NLSR process will keep the file open. Even if an external script renames the active file to “rotate” it, NLSR will still write its logs to the open file as identified by its inode.

#22 Updated by Ashlesh Gawande about 1 year ago

Junxiao Shi wrote:

I guess we can keep directing to a file as suggested in https://redmine.named-data.net/issues/3949#note-2
and have the system handle the rotation.

This won’t work because NLSR process will keep the file open. Even if an external script renames the active file to “rotate” it, NLSR will still write its logs to the open file as identified by its inode.

Okay, I see. Then it is better not direct to a file.

#23 Updated by Ashlesh Gawande about 1 year ago

  • Related to Task #4435: Correctly set log level and log re-direct after NLSR switches to ndn-cxx logger added

#24 Updated by Damian Coomes 12 months ago

  • Status changed from Code review to Closed
  • % Done changed from 90 to 100

#25 Updated by Ashlesh Gawande 12 months ago

  • Target version changed from v0.5.0 to Minor release 0.4.1

Also available in: Atom PDF