[
https://issues.apache.org/jira/browse/HADOOP-13396?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15417955#comment-15417955
]
Wei-Chiu Chuang commented on HADOOP-13396:
------------------------------------------
Hi [~xiaochen] thanks for the updated patch and thanks for clarification. I
made another round of review, and here's my comment:
* There is a test failure due to the change in audit message.
* user and impersonator could be null string. Would
JSONGenerator#writeStartObject get an NPE if the string is null? If not, and if
it prints "null" for null user/null impersonator, it could be hard to tell
whether it is a user/impersonator named 'null' or a null string.
* SimpleKMSAuditLogger#logAuditEvent:
if the status is UNAUTHORIZED, I think you can also use logAuditSimpleFormat()
to print audit log as well.
In fact, it looks like you only need special logic when status is OK.
* JsonKMSAuditLogger#logAuditEvent:
I think the same applies to this audit logger class. You should be able to call
logAuditSimpleFormat(). This will help simplify the logic.
Or if it can't, see if you can extract the lines in each switch-case, because
they are relatively long.
* The parameter opStatus in private void op(OpStatus opStatus, final KMS.KMSOp
op, final UserGroupInformation ugi, final String key, final String remoteHost,
final String extraMsg) { should be declared as final.
* With regard to the default logger, can we stick to the typical Hadoop
convention where we define
public static final String KMS_AUDIT_LOGGER_KEY_DEFAULT = “simple";
This simplifies the code in KMSAudit#initializeAuditLoggers().
> Add json format audit logging to KMS
> ------------------------------------
>
> Key: HADOOP-13396
> URL: https://issues.apache.org/jira/browse/HADOOP-13396
> Project: Hadoop Common
> Issue Type: New Feature
> Components: kms
> Reporter: Xiao Chen
> Assignee: Xiao Chen
> Attachments: HADOOP-13396.01.patch, HADOOP-13396.02.patch,
> HADOOP-13396.03.patch, HADOOP-13396.04.patch, HADOOP-13396.05.patch
>
>
> Currently, KMS audit log is using log4j, to write a text format log.
> We should refactor this, so that people can easily add new format audit logs.
> The current text format log should be the default, and all of its behavior
> should remain compatible.
> A json format log extension is added using the refactored API, and being
> turned off by default.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
