[#11170] fix(audit): Add ListEvent interface with resultCount to list events and surface count in audit log - #11199
Merged
Conversation
Code Coverage Report
Files |
jerryshao
force-pushed
the
fix/audit-list-events-11170
branch
from
May 25, 2026 06:58
5bf26ef to
8c4a66b
Compare
jerryshao
marked this pull request as ready for review
May 25, 2026 06:58
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the audit/event model for “list” operations by introducing a ListEvent marker interface (with resultCount()), updating list-success events and dispatchers to capture the returned item count, and surfacing {count=N} in SimpleAuditLogV2 output for improved auditability.
Changes:
- Add
ListEventinterface and implement it across list-success events (core + Iceberg REST server), with new preferred constructors carrying a result count and deprecated no-count constructors defaulting to-1. - Update core and Iceberg REST event dispatchers to pass actual list result sizes into events.
- Extend
SimpleAuditLogV2.toString()to include{count=N}in the custom-info field forListEvents, and add/adjust tests validating formatting and deprecated-constructor behavior.
Reviewed changes
Copilot reviewed 71 out of 71 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergViewOperations.java | Assert IcebergListViewEvent.resultCount() and add deprecated-constructor coverage. |
| iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java | Assert IcebergListTableEvent.resultCount() and add deprecated-constructor coverage. |
| iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceOperations.java | Assert IcebergListNamespacesEvent.resultCount() (including empty list) and add deprecated-constructor coverage. |
| iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergListViewEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergListTableEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergListNamespacesEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergViewEventDispatcher.java | Dispatch IcebergListViewEvent with response size. |
| iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableEventDispatcher.java | Dispatch IcebergListTableEvent with response size. |
| iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceEventDispatcher.java | Dispatch IcebergListNamespacesEvent with response size. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestTopicEvent.java | Adjust list mock to return empty array (non-null) for count calculation. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestTagEvent.java | Validate resultCount() on tag list events; add deprecated-constructor tests. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestTableEvent.java | Validate ListTableEvent.resultCount(); update mocks and add deprecated-constructor test. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestStatisticsEventDispatcher.java | Validate resultCount() for statistics list events; add deprecated-constructor tests. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestSchemaEvent.java | Validate ListSchemaEvent.resultCount(); update mocks and add deprecated-constructor test. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestPolicyEvent.java | Validate resultCount() for policy list events; add deprecated-constructor tests. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestPartitionEvent.java | Adjust list mocks to return empty arrays (non-null) for count calculation. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestMetalakeEvent.java | Validate ListMetalakeEvent.resultCount() including empty list; update mock and add deprecated-constructor test. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestJobEventDispatcher.java | Validate resultCount() for job list events; add deprecated-constructor tests. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestFunctionEvent.java | Adjust list mock to return empty array (non-null) for count calculation. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestFilesetEvent.java | Adjust list mock to return empty array (non-null) for count calculation. |
| core/src/test/java/org/apache/gravitino/listener/api/event/TestCatalogEvent.java | Validate ListCatalogEvent.resultCount() for both name/info listing; update mocks and add deprecated-constructor test. |
| core/src/test/java/org/apache/gravitino/audit/v2/TestSimpleAuditLogV2.java | Add comprehensive tests for {count=N} rendering/omission and merging with custom info. |
| core/src/test/java/org/apache/gravitino/audit/TestOperation.java | Update list-operation tests to use new list-success constructors and reorder assert args. |
| core/src/test/java/org/apache/gravitino/audit/TestFileAuditWriter.java | Add end-to-end audit line tests verifying {count=N} in formatted output. |
| core/src/main/java/org/apache/gravitino/listener/TopicEventDispatcher.java | Emit ListTopicEvent with returned count. |
| core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java | Emit tag-related list events with returned counts. |
| core/src/main/java/org/apache/gravitino/listener/TableEventDispatcher.java | Emit ListTableEvent with returned count. |
| core/src/main/java/org/apache/gravitino/listener/StatisticEventDispatcher.java | Emit statistics list events with returned counts. |
| core/src/main/java/org/apache/gravitino/listener/SchemaEventDispatcher.java | Emit ListSchemaEvent with returned count. |
| core/src/main/java/org/apache/gravitino/listener/PolicyEventDispatcher.java | Emit policy list events with returned counts. |
| core/src/main/java/org/apache/gravitino/listener/PartitionEventDispatcher.java | Emit partition list events with returned counts. |
| core/src/main/java/org/apache/gravitino/listener/ModelEventDispatcher.java | Emit ListModelEvent with returned count. |
| core/src/main/java/org/apache/gravitino/listener/MetalakeEventDispatcher.java | Emit ListMetalakeEvent with returned count. |
| core/src/main/java/org/apache/gravitino/listener/JobEventDispatcher.java | Emit job list events with returned counts. |
| core/src/main/java/org/apache/gravitino/listener/FunctionEventDispatcher.java | Emit ListFunctionEvent with returned count. |
| core/src/main/java/org/apache/gravitino/listener/FilesetEventDispatcher.java | Emit ListFilesetEvent with returned count. |
| core/src/main/java/org/apache/gravitino/listener/CatalogEventDispatcher.java | Emit ListCatalogEvent with returned count for both list APIs. |
| core/src/main/java/org/apache/gravitino/listener/api/event/stats/ListStatisticsEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/stats/ListPartitionStatisticsEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/policy/ListPolicyInfosForMetadataObjectEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/policy/ListPolicyInfosEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/policy/ListPoliciesEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/policy/ListMetadataObjectsForPolicyEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListUsersEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListUserNamesEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListTopicEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListTagsInfoForMetadataObjectEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListTagsInfoEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListTagsForMetadataObjectEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListTagsEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListTableEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListSchemaEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListRoleNamesEvent.java | Implement ListEvent; add count-aware constructors and deprecate no-count overloads. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListPartitionNamesEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListPartitionEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListModelVersionsEvent.java | Implement ListEvent with resultCount() derived from versions array length. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListModelVersionInfosEvent.java | Implement ListEvent with resultCount() derived from versions array length. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListModelEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListMetalakeEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListMetadataObjectsForTagEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListGroupsEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListGroupNamesEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListFilesetEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListEvent.java | New marker interface exposing resultCount() for list-success events. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListCatalogEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/job/ListJobTemplatesEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/job/ListJobsEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/api/event/function/ListFunctionInfosEvent.java | Implement ListEvent with resultCount() derived from returned array length. |
| core/src/main/java/org/apache/gravitino/listener/api/event/function/ListFunctionEvent.java | Implement ListEvent; add count-aware constructor + deprecated no-count constructor. |
| core/src/main/java/org/apache/gravitino/listener/AccessControlEventDispatcher.java | Emit user/group/role list events with returned counts. |
| core/src/main/java/org/apache/gravitino/audit/v2/SimpleAuditLogV2.java | Merge {count=N} into custom-info for ListEvents and omit it for -1. |
…log format - Problem 1: Rewrite FileAuditWriter to delegate file management (rotation, compression, retention) to Log4j2 via a dedicated SLF4J logger named 'gravitino.audit'. Add audit_file rolling appender to log4j2.properties.template (256 MB / daily rotation, gzip compression, 30-day retention). Deprecated writer properties (fileName, append, flushIntervalSecs) now emit WARN and are ignored. - Problem 3: Add RequestContext ThreadLocal to capture client IP on the servlet thread at Event construction time, so async listener threads read a field rather than accessing ThreadLocal. Add RequestContextFilter to set/clear the value around each request, honoring X-Forwarded-For for reverse-proxy setups. Apply the same X-Forwarded-For logic to IcebergRequestContext. - Problem 4: Fix SimpleAuditLogV2.toString() to use millisecond-precision timestamps (DateTimeFormatter, thread-safe) and append customInfo() as an 8th tab-separated field. - Update gravitino-server-config.md to document the Log4j2 migration path and mark the deprecated writer properties. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…o list events and surface count in audit log
Introduces a ListEvent marker interface so all list-operation success
events expose resultCount(). Dispatchers now pass the actual array/list
length when constructing events. SimpleAuditLogV2.toString() surfaces the
count as {count=N} in the custom-info field, omitting it when the
deprecated no-count constructors are used (resultCount() == -1).
Deprecated backward-compatible constructors are retained for existing
callers.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- SimpleAuditLogV2: apply customInfo before inserting count so the event-derived count cannot be overwritten by a caller-supplied 'count' key in customInfo() - TestSimpleAuditLogV2: remove ordering dependency from testListEventCountMergedWithExistingCustomInfo – check presence of both keys without asserting insertion order - TestIcebergNamespaceOperations / TestIcebergTableOperations / TestIcebergViewOperations: replace fully-qualified org.apache.gravitino.NameIdentifier usages with an import and short name per project style guidelines Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- SimpleAuditLogV2: replace LinkedHashMap with List<String> for customInfo formatting to tolerate duplicate keys; event-derived count and any user-supplied 'count' in customInfo now both appear in the audit field rather than one silently overwriting the other - SimpleAuditLogV2: simplify toString() into a single linear flow (remove the if/else branch between list and non-list paths) - ListModelVersionsEvent/ListModelVersionInfosEvent: guard resultCount() against null versions array to prevent NPE in audit logging - IcebergTableEventDispatcher / IcebergViewEventDispatcher / IcebergNamespaceEventDispatcher: move success event dispatch inside the try-catch and null-guard the identifiers/namespaces list before calling size(), preventing NPE propagation for successful operations - TestSimpleAuditLogV2: add test for duplicate 'count' key collision Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… count - ListViewEvent: implement ListEvent interface, add preferred constructor with viewCount, deprecate no-count constructor, add resultCount() - ViewEventDispatcher: pass identifiers.length to ListViewEvent - TestViewEvent: fix mock returning null for listViews (caused NPE with new identifiers.length call); add resultCount assertion - TestOperation: update to preferred ListViewEvent constructor Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jerryshao
force-pushed
the
fix/audit-list-events-11170
branch
from
May 26, 2026 03:24
3e13647 to
d780164
Compare
…ers and fix FQN imports - Add null guards (array/list != null ? array.length/list.size() : 0) for all list event dispatchers: AccessControl, Catalog, Fileset, Function, Job, Metalake, Model, Partition, Policy, Schema, Statistic, Table, Tag, Topic, View - Fix NPE in ListFunctionInfosEvent constructor when functionInfos is null - Fix FQN usages in TestSimpleAuditLogV2 by using import statements Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… and add resultCount assertions - Change all list event dispatchers to pass -1 (not 0) when result array is null, so resultCount() correctly signals "not captured" vs. "empty result" - Fix ListModelVersionsEvent, ListModelVersionInfosEvent, and ListFunctionInfosEvent to store the raw array (null-safe) and derive resultCount() from it, removing the incorrect null→empty coercion - Expand ListFilesEvent to accept resultCount in its primary constructor and deprecate the old no-count constructor (with @SuppressWarnings for errorprone) - Add resultCount() assertions to TestFilesetEvent, TestModelEvent, and TestFunctionEvent to cover the -1/actual-count contract Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
danhuawang
pushed a commit
to danhuawang/gravitino
that referenced
this pull request
Jun 8, 2026
…o list events and surface count in audit log (apache#11199) ### What changes were proposed in this pull request? Introduces a `ListEvent` marker interface that exposes `resultCount()` on all list-operation success event classes (core and Iceberg REST server). All event dispatchers are updated to pass the actual result count when constructing these events. `SimpleAuditLogV2.toString()` now surfaces the count as `{count=N}` in the custom-info audit-log field. Backward compatibility is preserved: the original no-count constructors are deprecated and delegate to `-1`; the audit formatter omits the count field when `resultCount()` returns `-1`. ### Why are the changes needed? Without the result count in audit logs it is impossible to distinguish an empty list response from a non-empty one, making audit trails less useful for security analysis and capacity planning. Fix: apache#11170 ### Does this PR introduce _any_ user-facing change? - New `ListEvent` interface added to the public `@DeveloperApi` surface. - All `List*Event` classes gain a new preferred constructor that accepts a result count; the old constructors are deprecated but still compile and run. - Audit log lines for list operations now include `{count=N}` in the last field. ### How was this patch tested? - `TestSimpleAuditLogV2`: new tests cover count in log output, zero count, null identifier, count merged with custom info, deprecated constructor omitting count, and non-list events. - `TestFileAuditWriter`: end-to-end log-line tests for `ListTableEvent`, `ListMetalakeEvent`, and `ListCatalogEvent`. - Existing dispatcher and event tests updated to use the new constructors. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
danhuawang
pushed a commit
to danhuawang/gravitino
that referenced
this pull request
Jun 9, 2026
…o list events and surface count in audit log (apache#11199) ### What changes were proposed in this pull request? Introduces a `ListEvent` marker interface that exposes `resultCount()` on all list-operation success event classes (core and Iceberg REST server). All event dispatchers are updated to pass the actual result count when constructing these events. `SimpleAuditLogV2.toString()` now surfaces the count as `{count=N}` in the custom-info audit-log field. Backward compatibility is preserved: the original no-count constructors are deprecated and delegate to `-1`; the audit formatter omits the count field when `resultCount()` returns `-1`. ### Why are the changes needed? Without the result count in audit logs it is impossible to distinguish an empty list response from a non-empty one, making audit trails less useful for security analysis and capacity planning. Fix: apache#11170 ### Does this PR introduce _any_ user-facing change? - New `ListEvent` interface added to the public `@DeveloperApi` surface. - All `List*Event` classes gain a new preferred constructor that accepts a result count; the old constructors are deprecated but still compile and run. - Audit log lines for list operations now include `{count=N}` in the last field. ### How was this patch tested? - `TestSimpleAuditLogV2`: new tests cover count in log output, zero count, null identifier, count merged with custom info, deprecated constructor omitting count, and non-list events. - `TestFileAuditWriter`: end-to-end log-line tests for `ListTableEvent`, `ListMetalakeEvent`, and `ListCatalogEvent`. - Existing dispatcher and event tests updated to use the new constructors. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Introduces a
ListEventmarker interface that exposesresultCount()on alllist-operation success event classes (core and Iceberg REST server). All event
dispatchers are updated to pass the actual result count when constructing these
events.
SimpleAuditLogV2.toString()now surfaces the count as{count=N}inthe custom-info audit-log field.
Backward compatibility is preserved: the original no-count constructors are
deprecated and delegate to
-1; the audit formatter omits the count field whenresultCount()returns-1.Why are the changes needed?
Without the result count in audit logs it is impossible to distinguish an
empty list response from a non-empty one, making audit trails less useful for
security analysis and capacity planning.
Fix: #11170
Does this PR introduce any user-facing change?
ListEventinterface added to the public@DeveloperApisurface.List*Eventclasses gain a new preferred constructor that accepts a resultcount; the old constructors are deprecated but still compile and run.
{count=N}in the last field.How was this patch tested?
TestSimpleAuditLogV2: new tests cover count in log output, zero count, nullidentifier, count merged with custom info, deprecated constructor omitting
count, and non-list events.
TestFileAuditWriter: end-to-end log-line tests forListTableEvent,ListMetalakeEvent, andListCatalogEvent.