KeDeng 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 3: (4 comments) Thanks for your reviews. 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 Done 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(), severity) : LOG(severity)) > 1. It's possible to use non-INFO severity level logs, isn't it? When I tested the custom implementation of the sink function, the output content was also recorded into the normal log file. In view of this, I only added the log level parameter here. 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: program_name_("kudu_master") { > nit: How about using std::make_unique instead? Done http://gerrit.cloudera.org:8080/#/c/21613/2/src/kudu/master/audit_logger.cc@63 PS2, Line 63: unique_ptr<RollingLog> l = > Will the AuditLogSink work well if open failed? You can CHECK_OK here or us Done -- 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: 3 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Thu, 01 Aug 2024 04:49:17 +0000 Gerrit-HasComments: Yes
