The one audit log that doesn't have a corresponding log for failure is logFsckEvent, though given that we get the ugi from the request it seems like that case could result in an ACE as well right?
the fsck audit event is logged before the fsck command is run, so it can't fail to generate the audit event. Also fsck is special in that it's implemented as a URL fetch, so I don't think the UGI is enforced. This is probably a bug, and the audit logging will need to be fixed when that bug is fixed.
Let's use fooInternal vs fooInt to match the existing "fooInternal" methods
That would collide with several existing uses: concatInternal, createSymlinkInternal, startFileInternal, renameToInternal, etc. I specifically chose a suffix not previously used to avoid code churn. Perhaps a different suffix than "Int" would convey this better, LMK if you have any good ideas.
Normally the checks are used before the method invocation if we're doing expensive things to create the args (eg lots of string concatenation) not to save the cost of the method invocation. Doesn't look like that's the case here (we're not constructing args) so we could just call logAuditEvent directly everywhere.
There are a bunch of uses of logAuditEvent that do need to check if audit logging is enabled before constructing log messages, etc. I considered refactoring them all and concluded that it was out of scope for this change. I decided not to change the existing idiom (verbose though it is) before refactoring all users of the interface, which should be a separate change.