jihoonson commented on a change in pull request #11078:
URL: https://github.com/apache/druid/pull/11078#discussion_r610896335
##########
File path:
server/src/main/java/org/apache/druid/server/audit/SQLAuditManagerConfig.java
##########
@@ -32,7 +33,10 @@
private boolean includePayloadAsDimensionInMetric = false;
@JsonProperty
- private long maxPayloadSizeBytes = -1;
+ private HumanReadableBytes maxPayloadSizeBytes =
HumanReadableBytes.valueOf(-1);
Review comment:
I think `maxPayloadSizeBytes` should not be a negative number than -1.
It would be nice to have a sanity check that checks the lower and upper bounds
(to avoid overflow) using `@HumanReadableBytesRange` as @FrankChen021 suggested
in https://github.com/apache/druid/pull/11078#discussion_r610285792.
##########
File path: core/src/main/java/org/apache/druid/audit/AuditManager.java
##########
@@ -32,17 +33,21 @@
* This String is the default message stored instead of the actual audit
payload if the audit payload size
* exceeded the maximum size limit configuration
*/
- String PAYLOAD_SKIP_MESSAGE = "Payload was not stored as the payload size
exceed limit configured by druid.audit.manager.maxPayloadSizeBytes";
+ String PAYLOAD_SKIP_MESSAGE = "Payload was not stored as its size exceeds
the limit [%d] configured by druid.audit.manager.maxPayloadSizeBytes";
Review comment:
Suggest `PAYLOAD_SKIP_MSG_FORMAT`.
##########
File path:
server/src/main/java/org/apache/druid/server/audit/SQLAuditManagerConfig.java
##########
@@ -31,6 +31,9 @@
@JsonProperty
private boolean includePayloadAsDimensionInMetric = false;
+ @JsonProperty
+ private long maxPayloadSizeBytes = -1;
Review comment:
@FrankChen021 oh you are right. There is a class for unit tests. Sorry,
a filter that excludes all test classes might be on in my intellij when I
searched last time :sweat:
##########
File path: docs/configuration/index.md
##########
@@ -338,8 +338,8 @@ Coordinator and Overlord log changes to lookups, segment
load/drop rules, dynami
|--------|-----------|-------|
|`druid.audit.manager.auditHistoryMillis`|Default duration for querying audit
history.|1 week|
|`druid.audit.manager.includePayloadAsDimensionInMetric`|Boolean flag on
whether to add `payload` column in service metric.|false|
-|`druid.audit.manager.maxPayloadSizeBytes`|The maximum size of audit payload,
in bytes, to store in Druid's metadata store audit table. If the size of audit
payload exceed this value, the audit log would be stored with a messaging
indicating that the payload was omitted instead. Setting `maxPayloadSizeBytes`
to -1 (default value) disables this check, meaning Druid will always store
audit payload regardless of it's size.|-1|
-
+|`druid.audit.manager.maxPayloadSizeBytes`|The maximum size of audit payload,
in bytes, to store in Druid's metadata store audit table. If the size of audit
payload exceed this value, the audit log would be stored with a messaging
indicating that the payload was omitted instead. Setting `maxPayloadSizeBytes`
to -1 (default value) disables this check, meaning Druid will always store
audit payload regardless of it's size. Human-readable format is supported, see
[here](human-readable-byte.md). |-1|
+|`druid.audit.manager.skipNullField`|If true, the audit payload stored in
metadata store will be exclude any field with null value. |false|
Review comment:
`will be exclude` -> `will exclude`?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
