Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/21613 )
Change subject: [log] Support logging audit logs to a separate file ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/21613/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21613/2//COMMIT_MSG@19 PS2, Line 19: [1]:https://google.github.io/glog/stable/sinks/ nit: How about 1. https://google.github.io/glog/stable/sinks/ http://gerrit.cloudera.org:8080/#/c/21613/2/src/kudu/master/audit_logger.h File src/kudu/master/audit_logger.h: http://gerrit.cloudera.org:8080/#/c/21613/2/src/kudu/master/audit_logger.h@94 PS2, Line 94: LOG_TO_SINK(AuditLogSink::GetSingleton(), INFO) : LOG(INFO)) 1. It's possible to use non-INFO severity level logs, isn't it? 2. When users view the common logs, it would be helpful if the "audit logs" are also logged there, then they are not needed to jump between the 2 kinds of log files. You can implement it like: #define LOG_AUDIT(severity, msg) \ do { \ if (FLAGS_master_audit_log_record_independently) { \ LOG_TO_SINK(AuditLogSink::GetSingleton(), severity) << msg; \ } \ LOG(severity) << msg; \ } while (false) Of course, pass the substituted strings to this macro. http://gerrit.cloudera.org:8080/#/c/21613/2/src/kudu/master/audit_logger.cc File src/kudu/master/audit_logger.cc: http://gerrit.cloudera.org:8080/#/c/21613/2/src/kudu/master/audit_logger.cc@62 PS2, Line 62: unique_ptr<RollingLog> l(new RollingLog(Env::Default(), log_dir_, program_name_, "AuditLog")); nit: How about using std::make_unique instead? http://gerrit.cloudera.org:8080/#/c/21613/2/src/kudu/master/audit_logger.cc@63 PS2, Line 63: WARN_NOT_OK(l->Open(), "Unable to open audit log"); Will the AuditLogSink work well if open failed? You can CHECK_OK here or use std::call_once in AuditLogSink::send(), or other approach. -- To view, visit http://gerrit.cloudera.org:8080/21613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie5323361befb456d91a12da7273865542f1d2430 Gerrit-Change-Number: 21613 Gerrit-PatchSet: 2 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Wed, 31 Jul 2024 16:39:58 +0000 Gerrit-HasComments: Yes
