Thanks for the update, Vrushali!
I should have said this earlier: normally we review patches against trunk first, since that's where the change must go before it goes anywhere else. We can only put this into 2.7 once it's also in trunk, branch-2, and branch-2.8, otherwise we risk releasing a fix/feature that disappears in later releases.
createSuccessLog should build on other versions rather than replicate the code. For example, now that we have an overload that takes the IP address as a string, the one that doesn't take an IP address should get the IP address as a string and call the other version.
Nit: ClientRMService#forceKillApplication is calling Server.getRemoteAddress three times which is wasteful, we should just cache this in a local.
Would RMAppKillByClientEvent be a more meaningful name than RMAppKillLogEvent?
Should we be passing around an InetAddress in the event and audit log overloads instead of a String for improved type safety? Not a must-fix, just wondering if it would be clearer.
I'm not sure we should log empty strings for values that aren't provided. It would probably be better to log "null" which is what it will already do for user names, for example, if we just let the null pass through. Not sure if audit log parsers will properly parse if there isn't some kind of corresponding token listed for a key.
Existing audit log code will not log a key for an IP address if it can't obtain it. This code will log an IP key with no value. May be a reason to pass the InetAddress through and let the audit logger decide whether to add the key if the value is non-null and it can obtain the address.
In the tests, why do we need a fooUser and a testUGI? I think we only need one of these. fooUser is created and then only used to get information to create testUGI, and I don't see why we can't just create testUGI directly. Also that UGI could be a static final "constant" in the test class rather than having each test method replicate the code.