somandal commented on code in PR #16694:
URL: https://github.com/apache/pinot/pull/16694#discussion_r2304737544
##########
pinot-common/src/main/java/org/apache/pinot/common/audit/AuditConfig.java:
##########
@@ -36,7 +36,7 @@ public final class AuditConfig {
private boolean _captureRequestPayload = false;
@JsonProperty("capture.request.headers")
- private boolean _captureRequestHeaders = false;
+ private String _captureRequestHeaders = "";
Review Comment:
nit: should this be renamed to `allowedRequestHeaders` instead since now it
isn't a boolean but a string? (same for the JsonProperty and the setter /
getter / log)
##########
pinot-common/src/main/java/org/apache/pinot/common/audit/AuditRequestProcessor.java:
##########
@@ -140,10 +165,15 @@ private AuditEvent.AuditRequestPayload
captureRequestPayload(ContainerRequestCon
}
final AuditConfig config = _configManager.getCurrentConfig();
- if (config.isCaptureRequestHeaders()) {
- MultivaluedMap<String, String> headers = requestContext.getHeaders();
- if (!headers.isEmpty()) {
- payload.setHeaders(toMap(headers));
+
+ Set<String> allowedHeaders =
parseAllowedHeaders(config.getCaptureRequestHeaders());
+ if (!allowedHeaders.isEmpty()) {
+ MultivaluedMap<String, String> allHeaders =
requestContext.getHeaders();
+ if (!allHeaders.isEmpty()) {
+ Map<String, Object> filteredHeaders = toMap(allHeaders,
allowedHeaders);
+ if (!filteredHeaders.isEmpty()) {
+ payload.setHeaders(filteredHeaders);
+ }
Review Comment:
is there a use case / scenario where we may want to capture all headers?
should wildcard `*` be allowed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
