Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZOOKEEPER-4648 Add audit log for request process result or response. #1962

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

curie71
Copy link

@curie71 curie71 commented Dec 13, 2022

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.

@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:

    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.

        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);

… request maybe better.

the err code after request processing is also important for audit.
@curie71
Copy link
Author

curie71 commented Dec 17, 2022

@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.

@curie71 curie71 changed the title ZOOKEEPER-4648 FinalRequestProcessor addAuditLog after the process of request maybe better. ZOOKEEPER-4648 Add audit log for request process result or response. Dec 18, 2022
@eolivelli
Copy link
Contributor

can you please rebase ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants