-
Notifications
You must be signed in to change notification settings - Fork 5
Add Support for Query Params For Audit Log #90
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
base: dev
Are you sure you want to change the base?
Add Support for Query Params For Audit Log #90
Conversation
proud-parselmouth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just 1 comment.
|
|
||
| // Verify that query parameters are included in the request path | ||
| String requestPath = auditLog.getRequestPath(); | ||
| Assert.assertTrue(requestPath.contains("?"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should we just write one assert testing if the auditLog.getRequestPath() matches with the expected requested path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed!
proud-parselmouth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
bellatrix007
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but can we remove the internal LinkedIn related info like CICP ticket number in the PR description.
Issues
When we see logs in Kusto, we see only request paths but query params (if any) were being removed.
Description
AuditLogFilter.javasets value forAUDIT_LOGkey.Which is passed to
logger.write()method.This method is overwritten by
HelixRestAuditLoggerandDefaultHelixRestAuditLogger.java.Tests
TestClusterAccessor.testAuditLogIncludesQueryParamsmvn test -pl helix-rest -Dtest=TestClusterAccessor#testAuditLogIncludesQueryParams 2>&1Estimation for Log Volume Increase
So
AuditLogconsists of roughly 36.4%.By Paretto principle of 80-20 say 80% of requests have query parameters.
And also assuming having query params increases request path length by 40 chars (In UTF-8 if a char is 1B then 40B per request path).
Volume Increase Calculation
Considering disk compression of 5x:
3.6 GB/day