zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From anmolnar <...@git.apache.org>
Subject [GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Date Mon, 23 Apr 2018 15:30:00 GMT
Github user anmolnar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r183433194
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -250,30 +267,36 @@ public void processRequest(Request request) {
                     lastOp = "CREA";
                     rsp = new Create2Response(rc.path, rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_CREATE, rc.path, null, err);
                     break;
                 }
                 case OpCode.delete:
                 case OpCode.deleteContainer: {
                     lastOp = "DELE";
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_DELETE, rc.path, null, err);
                     break;
                 }
                 case OpCode.setData: {
                     lastOp = "SETD";
                     rsp = new SetDataResponse(rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_SETDATA, rc.path, null,
err);
                     break;
                 }
                 case OpCode.reconfig: {
                     lastOp = "RECO";
                     rsp = new GetDataResponse(((QuorumZooKeeperServer)zks).self.getQuorumVerifier().toString().getBytes(),
rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_RECONFIG, rc.path, null,
err);
                     break;
                 }
                 case OpCode.setACL: {
                     lastOp = "SETA";
                     rsp = new SetACLResponse(rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_SETACL, rc.path, getACLs(request),
    --- End diff --
    
    I believe this approach has some performance impact even if audit logging is disabled.
The flag gets checked within the method, therefore `getACLs()` will be evaluated even if there's
no need for the result.
    Passing only `request` and calling `getACLs()` from `addAuditLog()` would be slightly
faster.  


---

Mime
View raw message