[
https://issues.apache.org/jira/browse/HADOOP-13396?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiao Chen updated HADOOP-13396:
-------------------------------
Attachment: HADOOP-13396.07.patch
Thanks again for the comments, Andrew and Wei-Chiu!
As chatted offline, I'd like to make this patch simple, to solely refactor the
kmsaudit logger to be pluggable. Will create a new jira for the Json addition.
Patch 7 is toward this direction.
I will address all json related comments in the new jira, this leaves us with:
bq. Is KMSAuditLogger interface really InterfaceAudience.Public?
Changed it to Private.
bq. We might still consider using the classname as configuration though.... I'd
recommend splitting out each audit logger implementation as a separate class.
Separated the classes, with initialization done by reflection.
bq. Could we add a big warning about the importance of audit logger output
compatibility to KMSAuditLogger's class javadoc? We could use similar reminders
in the logger implementations.
I added warning notes I feel pretty serious, to both the interface's and
implementation's header.
bq. kms-site.xml description should say how this takes a comma-separated list.
done.
bq. Are multiple audit loggers unit tested?
Manually tested, I think they're separate enough to be tested on their own.
Feel free to speak out what testing is in your thoughts though, we could add. :)
bq. What happens if the same value (e.g. "simple") is specified multiple times?
{{Configuration#getTrimmedStringCollection}} calls into
{{StringUtils#getTrimmedStringCollection}} which removes duplicates. Added a
comment in KMSAudit for this.
bq. adding url in the message, and it being inconsistent
I reverted any change to the message. I plan to see what the recent (audit) log
compatibility thread concludes, and take actions based on that. Will assume
nothing can be changed for now.
bq. The multiple uses of System.currentTimeMillis() is suspect. It means with
multiple audit loggers, they could have different times.
Good catch, added an {{endTime}} field on the event to address.
bq. Could we make an effort to use the same key names as the current audit
logger? e.g. "op" instead of "operation", "user" instead of "username". This
will make life easier for consumers.
This is for the json logger right? I'll defer this to the new jira.
> 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,
> HADOOP-13396.06.patch, HADOOP-13396.07.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: [email protected]
For additional commands, e-mail: [email protected]
