nevzheng opened a new issue, #12691: URL: https://github.com/apache/gravitino/issues/12691
## Summary Gravitino's table events can carry structured audit facts, but only the code that constructs an event can populate them — and for table events that is always the outermost dispatcher. An inner `TableDispatcher` layer that applied a policy, enforced a validation, or made a decision has no way to get that fact into the `CreateTableEvent` the outer layer publishes. That leaves an audit record that says an operation happened without saying anything a middle layer knew about it. The layer's only options today are to publish a separate event, which breaks the one-operation-one-event shape audit consumers rely on and forces them to correlate two records, or to drop the fact. ## Desired outcome An inner dispatcher layer can attach `Map<String, String>` audit facts to the terminal table event the outer dispatcher already emits, so a single audit record carries both what happened and what the chain knew about it. Enrichment should change what an event carries and nothing else. Dispatch semantics, error propagation, and the caller-visible result of the operation should be identical whether or not facts were attached. ## Scope and boundaries In scope: the table lifecycle events — `CreateTableEvent`, `LoadTableEvent`, `AlterTableEvent`, and their `*FailureEvent` counterparts — and whatever hand-off mechanism lets an inner layer supply facts to `TableEventDispatcher`. Out of scope, and worth stating because it is easy to conflate: whether a listener that throws while consuming a *success* event should fail the caller. Today `EventBus.dispatchPostEvent` lets that exception propagate, while `dispatchFailureEvent` swallows it to avoid masking an original error. Whether to isolate success-path listener failures is a reasonable question, but it is a question about `EventBus` semantics that applies equally to events carrying no custom info. Bundling it here would change existing behavior under cover of an additive feature. Also out of scope for a first change: the other fourteen dispatchers. The same gap exists in each, but doing all fifteen at once makes the first review harder than it needs to be. ## Evidence The event API already anticipates this kind of enrichment; table events are the gap. - `BaseEvent` declares `customInfo()` returning an empty map, marked `@since 0.8.0` (`core/src/main/java/org/apache/gravitino/listener/api/event/BaseEvent.java:131`). - Two server-layer events already override it, and both are cases where the *reason* for an outcome was the point: `AuthorizationDenialFailureEvent` and `HttpRequestFailureEvent` (`core/src/main/java/org/apache/gravitino/listener/api/event/server/`). - Both audit formatters already serialize it, with redaction applied on the way out (`core/src/main/java/org/apache/gravitino/audit/JsonAuditFormatter.java:69`, `core/src/main/java/org/apache/gravitino/audit/v2/SimpleAuditLogV2.java:101`, `core/src/main/java/org/apache/gravitino/audit/AuditLogRedactor.java`). So the consumption side is built end to end. Table events inherit the empty default with no way to populate it. ## Supporting details A proposed approach, not a decided one: - Add a constructor to `TableEvent` and `TableFailureEvent`, and a matching overload to the six concrete table events, accepting custom info. Existing constructors delegate with an empty map, so nothing constructing these events today changes behavior. - Give a contributing layer somewhere to stash facts for the current request, and have `TableEventDispatcher` consume them when it builds the terminal event. The contributing layer and the emitting layer are not in the same call frame, and the `TableDispatcher` signature cannot carry the value, so the hand-off needs a home. `RequestContext` already holds per-request state of exactly this shape, which makes extending it the smallest change — but a thread-confined hand-off is the debatable part of the design and deserves scrutiny. Alternatives worth weighing include a dedicated contributor interface the outer dispatcher queries explicitly, or widening `TableDispatcher` to return a richer result. Both are larger and touch more API surface. cc @roryqi @jerryshao @lasdf Nevin Sent from my 🤖 (Cursor) -- 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]
