You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2022/12/13 08:24:37 UTC

[GitHub] [zookeeper] curie71 opened a new pull request, #1962: ZOOKEEPER-4648 FinalRequestProcessor addAuditLog after the process of request maybe better.

curie71 opened a new pull request, #1962:
URL: https://github.com/apache/zookeeper/pull/1962

   ZOOKEEPER-4648 FinalRequestProcessor addAuditLog before the process of request and make failedTxn=false. But I think failedTxn should be true if the request can not pass the checkACL and throw KeeperException or other exceptions, **since the err code after request processing is also important for audit.**
   ```java
   @param failedTxn whether audit is being done failed transaction for normal transaction
   
   public void processRequest(Request request) {
           ......
           Code err = Code.OK;
           try {
               ......
               AuditHelper.addAuditLog(request, rc);
   
               switch (request.type) {
               ......
               case OpCode.getAllChildrenNumber: {
                   lastOp = "GETACN";
                   ......
                   zks.checkACL(
                       request.cnxn,
                       zks.getZKDatabase().aclForNode(n),
                       ZooDefs.Perms.READ,
                       request.authInfo,
                       path,
                       null);
                   ......
                   break;
               }
               ......
               }
           } catch (SessionMovedException e) {
               ......
           } catch (KeeperException e) {
               err = e.code();
           } catch (Exception e) {
               ......
           }
   ```
   if the failedTxn == true or the rc.err != Code.OK, the log result will be FAILURE: 
   ```java
       private static Result getResult(ProcessTxnResult rc, boolean failedTxn) {
           if (failedTxn) {
               return Result.FAILURE;
           } else {
               return rc.err == KeeperException.Code.OK.intValue() ? Result.SUCCESS : Result.FAILURE;
           }
       }
   ```
   So we could add audit log after request processing and record the err code like below, the log info maybe more accurate. 
   ```java
           Code err = Code.OK;
           try { 
                ......
           } catch (SessionMovedException e) {
               ......
           } catch (KeeperException e) {
               err = e.code();
           } catch (Exception e) {
               ......
           }
           rc.err = err.intValue();
           AuditHelper.addAuditLog(request, rc);
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zookeeper] eolivelli commented on pull request #1962: ZOOKEEPER-4648 Add audit log for request process result or response.

Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on PR #1962:
URL: https://github.com/apache/zookeeper/pull/1962#issuecomment-1428308834

   can you please rebase ?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zookeeper] curie71 commented on pull request #1962: ZOOKEEPER-4648 FinalRequestProcessor addAuditLog after the process of request maybe better.

Posted by GitBox <gi...@apache.org>.
curie71 commented on PR #1962:
URL: https://github.com/apache/zookeeper/pull/1962#issuecomment-1356270372

   @dobozysaurus Could you please to take a look at this issue?
   I found only the request audit log is recorded here in zookeeper.
   But I think "the err code after request processing" or a "response log" is also important for audit.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org