You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Don Bosco Durai <bo...@apache.org> on 2015/04/15 03:08:01 UTC
Review Request 33203: Implement reliable streaming audits to
configurable destinations
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33203/
-----------------------------------------------------------
Review request for ranger, Alok Lal, Madhan Neethiraj, and Selvamohan Neethiraj.
Bugs: RANGER-397
https://issues.apache.org/jira/browse/RANGER-397
Repository: ranger
Description
-------
Implement reliable streaming audits to configurable destinations
Diffs
-----
.gitignore 2c746ed
agents-audit/src/main/java/org/apache/ranger/audit/model/AuditEventBase.java 82fcab8
agents-audit/src/main/java/org/apache/ranger/audit/model/AuthzAuditEvent.java d0c1526
agents-audit/src/main/java/org/apache/ranger/audit/provider/AsyncAuditProvider.java 5da5064
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditAsyncQueue.java PRE-CREATION
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditBatchProcessor.java PRE-CREATION
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditDestination.java PRE-CREATION
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditFileSpool.java PRE-CREATION
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditMessageException.java PRE-CREATION
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditProvider.java 47c2d7f
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditProviderFactory.java bb8fa6d
agents-audit/src/main/java/org/apache/ranger/audit/provider/BaseAuditProvider.java a068b8f
agents-audit/src/main/java/org/apache/ranger/audit/provider/BufferedAuditProvider.java be32519
agents-audit/src/main/java/org/apache/ranger/audit/provider/DbAuditProvider.java 7414a7a
agents-audit/src/main/java/org/apache/ranger/audit/provider/DummyAuditProvider.java a6e3ef1
agents-audit/src/main/java/org/apache/ranger/audit/provider/FileAuditDestination.java PRE-CREATION
agents-audit/src/main/java/org/apache/ranger/audit/provider/HDFSAuditDestination.java PRE-CREATION
agents-audit/src/main/java/org/apache/ranger/audit/provider/LocalFileLogBuffer.java cdc4d53
agents-audit/src/main/java/org/apache/ranger/audit/provider/Log4jAuditProvider.java 0d0809a
agents-audit/src/main/java/org/apache/ranger/audit/provider/LogDestination.java 44e94ad
agents-audit/src/main/java/org/apache/ranger/audit/provider/MiscUtil.java 17230b2
agents-audit/src/main/java/org/apache/ranger/audit/provider/MultiDestAuditProvider.java 1eec345
agents-audit/src/main/java/org/apache/ranger/audit/provider/hdfs/HdfsAuditProvider.java 620951c
agents-audit/src/main/java/org/apache/ranger/audit/provider/hdfs/HdfsLogDestination.java 6b5cb4b
agents-audit/src/main/java/org/apache/ranger/audit/provider/kafka/KafkaAuditProvider.java 0ec8790
agents-audit/src/main/java/org/apache/ranger/audit/provider/solr/SolrAuditProvider.java 1b463e6
security-admin/.gitignore 4a3ed53
security-admin/pom.xml 3220886
security-admin/src/test/java/org/apache/ranger/audit/TestAuditProcessor.java PRE-CREATION
Diff: https://reviews.apache.org/r/33203/diff/
Testing
-------
Added JUnits to test the features
Thanks,
Don Bosco Durai
Re: Review Request 33203: Implement reliable streaming audits to
configurable destinations
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33203/#review80299
-----------------------------------------------------------
Bosco - I haven't completed reviewing yet. Here are the comments so far. I will need couple of more hours to complete the review; will do that by tomorrow.
agents-audit/src/main/java/org/apache/ranger/audit/model/AuthzAuditEvent.java
<https://reviews.apache.org/r/33203/#comment130109>
Couldn't eventTime field be used instead of adding field seq_num?
agents-audit/src/main/java/org/apache/ranger/audit/model/AuthzAuditEvent.java
<https://reviews.apache.org/r/33203/#comment130110>
eventCount might be a better name, instead of frequentCount.
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditAsyncQueue.java
<https://reviews.apache.org/r/33203/#comment130113>
return from log(event) is dropped here. It shoud instead be propagated up.
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditAsyncQueue.java
<https://reviews.apache.org/r/33203/#comment130121>
Please set consumerThread=null here.
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditAsyncQueue.java
<https://reviews.apache.org/r/33203/#comment130116>
It might be better to not clear eventList here - in case the consumer keeps a refernece for later use.
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditAsyncQueue.java
<https://reviews.apache.org/r/33203/#comment130117>
On receiving InterruptedException, shouldn't the while(true) loop terminate? irrespetive of isDrain()/queue.isEmpty()?
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditBatchProcessor.java
<https://reviews.apache.org/r/33203/#comment130122>
Please set consumerThread=null here.
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditProvider.java
<https://reviews.apache.org/r/33203/#comment130128>
Given that some customers would have added custom AuditProviders, for example to send Ranger Audit logs to their desired destination, it will be important to keep this interface backward compatible i.e. pretty much no change:-(
agents-audit/src/main/java/org/apache/ranger/audit/provider/BaseAuditProvider.java
<https://reviews.apache.org/r/33203/#comment130123>
BaseAuditProvider has become too complicated (to be called as Base)! fileSpooler, consumer, drain, etc.. It might be cleaner to move all the new functionalty to another new class; and keep BaseAuditProvider simple.
agents-audit/src/main/java/org/apache/ranger/audit/provider/BufferedAuditProvider.java
<https://reviews.apache.org/r/33203/#comment130129>
Shouldn't the return be false if line #53 returns false?
agents-audit/src/main/java/org/apache/ranger/audit/provider/BufferedAuditProvider.java
<https://reviews.apache.org/r/33203/#comment130130>
Shouldn't the return from log(event) be propagated, instead of returning true?
agents-audit/src/main/java/org/apache/ranger/audit/provider/BufferedAuditProvider.java
<https://reviews.apache.org/r/33203/#comment130131>
return false? Or propogate the return from logJSON(event)?
agents-audit/src/main/java/org/apache/ranger/audit/provider/DbAuditProvider.java
<https://reviews.apache.org/r/33203/#comment130132>
Shouldn't the return from log(event) be propagated, instead of returning true?
agents-audit/src/main/java/org/apache/ranger/audit/provider/DbAuditProvider.java
<https://reviews.apache.org/r/33203/#comment130133>
return false? Or propogate the return from logJSON(event)?
- Madhan Neethiraj
On April 15, 2015, 1:07 a.m., Don Bosco Durai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33203/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 1:07 a.m.)
>
>
> Review request for ranger, Alok Lal, Madhan Neethiraj, and Selvamohan Neethiraj.
>
>
> Bugs: RANGER-397
> https://issues.apache.org/jira/browse/RANGER-397
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Implement reliable streaming audits to configurable destinations
>
>
> Diffs
> -----
>
> .gitignore 2c746ed
> agents-audit/src/main/java/org/apache/ranger/audit/model/AuditEventBase.java 82fcab8
> agents-audit/src/main/java/org/apache/ranger/audit/model/AuthzAuditEvent.java d0c1526
> agents-audit/src/main/java/org/apache/ranger/audit/provider/AsyncAuditProvider.java 5da5064
> agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditAsyncQueue.java PRE-CREATION
> agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditBatchProcessor.java PRE-CREATION
> agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditDestination.java PRE-CREATION
> agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditFileSpool.java PRE-CREATION
> agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditMessageException.java PRE-CREATION
> agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditProvider.java 47c2d7f
> agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditProviderFactory.java bb8fa6d
> agents-audit/src/main/java/org/apache/ranger/audit/provider/BaseAuditProvider.java a068b8f
> agents-audit/src/main/java/org/apache/ranger/audit/provider/BufferedAuditProvider.java be32519
> agents-audit/src/main/java/org/apache/ranger/audit/provider/DbAuditProvider.java 7414a7a
> agents-audit/src/main/java/org/apache/ranger/audit/provider/DummyAuditProvider.java a6e3ef1
> agents-audit/src/main/java/org/apache/ranger/audit/provider/FileAuditDestination.java PRE-CREATION
> agents-audit/src/main/java/org/apache/ranger/audit/provider/HDFSAuditDestination.java PRE-CREATION
> agents-audit/src/main/java/org/apache/ranger/audit/provider/LocalFileLogBuffer.java cdc4d53
> agents-audit/src/main/java/org/apache/ranger/audit/provider/Log4jAuditProvider.java 0d0809a
> agents-audit/src/main/java/org/apache/ranger/audit/provider/LogDestination.java 44e94ad
> agents-audit/src/main/java/org/apache/ranger/audit/provider/MiscUtil.java 17230b2
> agents-audit/src/main/java/org/apache/ranger/audit/provider/MultiDestAuditProvider.java 1eec345
> agents-audit/src/main/java/org/apache/ranger/audit/provider/hdfs/HdfsAuditProvider.java 620951c
> agents-audit/src/main/java/org/apache/ranger/audit/provider/hdfs/HdfsLogDestination.java 6b5cb4b
> agents-audit/src/main/java/org/apache/ranger/audit/provider/kafka/KafkaAuditProvider.java 0ec8790
> agents-audit/src/main/java/org/apache/ranger/audit/provider/solr/SolrAuditProvider.java 1b463e6
> security-admin/.gitignore 4a3ed53
> security-admin/pom.xml 3220886
> security-admin/src/test/java/org/apache/ranger/audit/TestAuditProcessor.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33203/diff/
>
>
> Testing
> -------
>
> Added JUnits to test the features
>
>
> Thanks,
>
> Don Bosco Durai
>
>