-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat: add configurations around endpoint logging #8249
Conversation
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
try { | ||
// do not log the query that was sent with the URI, since this may contain | ||
// sensitive user information | ||
final URI uriWithQueryString = new URI(routingContext.request().uri()); |
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.
uriWithoutQueryString?
Also, I think you can just ask for routingContext.request().path() if you want just the path, rather than having to construct it by hand.
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.
nice - thanks for the tip
} | ||
} | ||
|
||
if (endpointFilter.isPresent() && endpointFilter.get().matcher(uri).matches()) { |
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.
Filter is a bit confusing because some filters take functions which retain their elements rather than remove them. Not sure if I have a suggestion for a name. KSQL_ENDPOINT_LOGGING_SKIP_ENDPOINT_CONFIG
?
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 went with KSQL_ENDPOINT_LOGGING_IGNORED_PATHS_REGEX
Description
Endpoint logging potentially contains sensitive information. This patch allows some more configuration to control this behavior by introducing two configurations:
Testing done
Unit testing
Reviewer checklist