Hi,
If you have time, can you please review the patch?
I have done self review, a few things I think it needs to update
- The redundant headers file in mds_log.cc that were needed for MdsLog
class should be removed
- The TraceLog::Init needs to return if the Init() has been already done.
- With the support of #2838, the log/trace can be written if application
is running as non-root user, the new OSAF_LOCAL_NODE_LOG env also needs
to be documented in a PR doc.
- Another thing, if application uses 2 or more services (say NTF, AMF)
and enables both NTFSV_TRACE_PATHNAME and AVA_TRACE_PATHNAME as in two
different paths, but only the first variable path is used for both
agents' tracing. This seems to be there before #2306, changing this may
cause a NBC, so maybe just document it somehow
Regards,
Minh
On 12/04/18 09:12, Minh Chau wrote:
Unify TraceLog and MdsLog class to one class (TraceLog) so it can be
used as a common log client.
Add new instance TraceLog for OpenSAF logging to local file, which can
be enabled/disabled via environment variable OSAF_LOCAL_NODE_LOG
---
src/base/logtrace.cc | 167 ++-
src/base/logtrace.h | 50 +--
src/mds/mds_log.cc | 114 +++
3 files changed, 140 insertions(+), 191 deletions(-)
diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc
index b046fab..857e31c 100644
--- a/src/base/logtrace.cc
+++ b/src/base/logtrace.cc
@@ -36,15 +36,10 @@
#include
#include
#include
-#include "base/buffer.h"
-#include "base/conf.h"
-#include "base/log_message.h"
-#include "base/macros.h"
-#include "base/mutex.h"
+#include "base/getenv.h"
#include "base/ncsgl_defs.h"
#include "base/osaf_utility.h"
#include "base/time.h"
-#include "base/unix_client_socket.h"
#include "dtm/common/osaflog_protocol.h"
namespace global {
@@ -55,65 +50,38 @@ const char *const prefix_name[] = {"EM", "AL", "CR", "ER", "WA",
"NO", "IN",
"T6", "T7", "T8", ">>", "<<"};
char *msg_id;
int logmask;
+const char* osaf_log_file = "osaf.log";
+bool enable_osaf_log = false;
} // namespace global
-class TraceLog {
- public:
- static bool Init();
- static void Log(base::LogMessage::Severity severity, const char *fmt,
- va_list ap);
-
- private:
- TraceLog(const std::string , const std::string _name,
- uint32_t proc_id, const std::string _id,
- const std::string _name);
- void LogInternal(base::LogMessage::Severity severity, const char *fmt,
- va_list ap);
- static constexpr const uint32_t kMaxSequenceId = uint32_t{0x7fff};
- static TraceLog *instance_;
- const base::LogMessage::HostName fqdn_;
- const base::LogMessage::AppName app_name_;
- const base::LogMessage::ProcId proc_id_;
- const base::LogMessage::MsgId msg_id_;
- uint32_t sequence_id_;
- base::UnixClientSocket log_socket_;
- base::Buffer<512> buffer_;
- base::Mutex mutex_;
-
- DELETE_COPY_AND_MOVE_OPERATORS(TraceLog);
-};
-
-TraceLog *TraceLog::instance_ = nullptr;
-
-TraceLog::TraceLog(const std::string , const std::string _name,
- uint32_t proc_id, const std::string _id,
- const std::string _name)
-: fqdn_{base::LogMessage::HostName{fqdn}},
- app_name_{base::LogMessage::AppName{app_name}},
- proc_id_{base::LogMessage::ProcId{std::to_string(proc_id)}},
- msg_id_{base::LogMessage::MsgId{msg_id}},
- sequence_id_{1},
- log_socket_{socket_name, base::UnixSocket::kBlocking},
- buffer_{},
- mutex_{} {}
-
-bool TraceLog::Init() {
- if (instance_ != nullptr) return false;
- char app_name[49];
- char pid_path[1024];
+static TraceLog gl_trace;
+static TraceLog gl_osaflog;
+TraceLog::TraceLog()
+: sequence_id_{1}, buffer_{} {
+ mutex_ = nullptr;
+ log_socket_ = nullptr;
+}
+
+TraceLog::~TraceLog() {
+ if (mutex_) delete mutex_;
+ if (log_socket_) delete log_socket_;
+}
+bool TraceLog::Init(const char *msg_id, WriteMode mode) {
+ char app_name[NAME_MAX];
+ char pid_path[PATH_MAX];
uint32_t process_id = getpid();
char *token, *saveptr;
char *pid_name = nullptr;
- memset(app_name, 0, 49);
- memset(pid_path, 0, 1024);
+ memset(app_name, 0, NAME_MAX);
+ memset(pid_path, 0, PATH_MAX);
snprintf(pid_path, sizeof(pid_path), "/proc/%" PRIu32 "/cmdline", process_id);
FILE *f = fopen(pid_path, "r");
if (f != nullptr) {
size_t size;
-size = fread(pid_path, sizeof(char), 1024, f);
+size = fread(pid_path, sizeof(char), PATH_MAX, f);
if (size > 0) {
if ('\n' == pid_path[size - 1]) pid_path[size - 1] = '\0';
}
@@ -131,19 +99,28 @@ bool TraceLog::Init() {
}
base::Conf::InitFullyQualifiedDomainName();
const std::string = base::Conf::FullyQualifiedDomainName();
- instance_ = new TraceLog{fqdn, app_name, process_id, global::msg_id,
-