github-actions[bot] commented on code in PR #63865:
URL: https://github.com/apache/doris/pull/63865#discussion_r3345877669
##########
fe/fe-core/src/main/java/org/apache/doris/qe/AuditLogHelper.java:
##########
@@ -279,6 +290,15 @@ private static void logAuditLogImpl(ConnectContext ctx,
String origStmt, Stateme
.setCloudCluster(Strings.isNullOrEmpty(cluster) ? "UNKNOWN" :
cluster)
.setWorkloadGroup(ctx.getWorkloadGroupName());
+ // load_label and txn_id for transactional statements
(INSERT/UPDATE/DELETE)
Review Comment:
This only reads `insertResult` from the local `ConnectContext`, but normal
`INSERT`/`UPDATE`/`DELETE`/`MERGE_INTO` on a non-master FE are forwarded
because these commands implement `ForwardWithSync`. The master-side
`proxyExecute()` runs the DML and populates the proxy context's insert result,
but `TMasterOpResult` only returns packet/status/affected rows/txnLoadInfo, and
`FEOpExecutor.execute()` never copies label/txnId back into the original
context. The observer/follower then audits the statement here with empty
`load_label` and `txn_id = -1` even though the DML succeeded. Please propagate
the insert result through the forward response, or otherwise set these audit
fields from the forwarded execution result before logging.
##########
fe/fe-core/src/main/java/org/apache/doris/plugin/audit/LoadAuditEvent.java:
##########
@@ -65,8 +63,8 @@ public AuditEventBuilder setJobId(long jobId) {
return this;
}
- public AuditEventBuilder setLabel(String label) {
- auditEvent.label = label;
+ public AuditEventBuilder setLoadLabel(String loadLabel) {
Review Comment:
`AuditLogBuilder.auditLoadLog()` still filters and emits
`LOAD_ANNONATION_NAMES`, which contains `Label`, but this PR removed the only
`@AuditField(value = "Label")` from `LoadAuditEvent` and stores the value in
inherited `LoadLabel` instead. Because the builder map is keyed by annotation
value, every `LOAD_SUCCEED` line in `fe.load.audit.log` will now contain
`|Label=null` even when the load has a label. Please keep a `Label` audit field
for load audit events or update the load audit builder/output name together
with this rename.
##########
fe/fe-core/src/main/java/org/apache/doris/plugin/audit/StreamLoadAuditEvent.java:
##########
@@ -64,8 +62,8 @@ public AuditEventBuilder setEventType(EventType eventType) {
return this;
}
- public AuditEventBuilder setLabel(String label) {
- auditEvent.label = label;
+ public AuditEventBuilder setLoadLabel(String loadLabel) {
Review Comment:
The stream-load text audit path has the same regression:
`AuditLogBuilder.auditStreamLoadLog()` still looks for annotation `Label`, but
this class no longer exposes any `@AuditField(value = "Label")`. As a result
`fe.stream_load.audit.log` will print `|Label=null` for completed stream loads.
Please keep the legacy `Label` audit field here or update the stream-load audit
builder/output contract consistently.
--
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]
