nevzheng opened a new pull request, #12723:
URL: https://github.com/apache/gravitino/pull/12723

   ## Summary
   
   An inner Iceberg REST dispatcher layer can now attach `Map<String, String>` 
audit facts to the terminal create/update/load event that 
`IcebergTableEventDispatcher` already emits, so one operation still produces 
one audit record and that record can say what the dispatch chain knew about it.
   
   This is the Iceberg REST counterpart of #12692. The change is deliberately 
confined to carrying facts. Dispatch semantics are untouched: events still go 
out through `eventBus.dispatchEvent`, and a listener exception propagates 
exactly as it does on `main`, whether or not extras were stashed. 
Listener-failure isolation is out of scope.
   
   ## Context
   
   Iceberg REST table events already expose `customInfo()`, but today that map 
is only the request HTTP headers (`IcebergEvent.customInfo()` returns 
`icebergRequestContext.httpHeaders()`). An inner dispatcher that applied a 
policy or enforced a validation had no way to get that fact into the terminal 
Iceberg create/update/load event the outer layer publishes.
   
   The layer's only options today are to publish a separate event, which breaks 
the one-operation-one-event shape audit consumers rely on, or to drop the fact. 
#12692 already landed the same hand-off for Gravitino `TableEventDispatcher` 
via `RequestContext.setAuditExtras` / `takeAuditExtras`. Iceberg REST has the 
same layered shape; this reuses that stash rather than adding a parallel 
mechanism.
   
   ## Changes
   
   - **New**: `IcebergRequestContext` gains an immutable extras map 
(`auditExtras` / `withAuditExtras`). Extras are not part of `httpHeaders()`.
   - **New**: `IcebergEvent` and `IcebergFailureEvent` `customInfo()` is 
headers ∪ extras. When extras are empty, `customInfo()` still returns today's 
header-only map.
   - **New**: `IcebergTableEventDispatcher` takes 
`RequestContext.takeAuditExtras()` on create/update/load success and failure 
and attaches them to the emitted Iceberg context. The take is destructive, and 
`RequestContext.clear()` still removes the stash at request teardown.
   - **Unchanged**: drop/list/rename and every other Iceberg operation. 
`EventBus` is untouched. Error propagation and operation results are untouched.
   - **Deferred**: listener-failure isolation. Today Iceberg dispatch lets a 
success-path listener exception propagate. Whether to isolate that is a 
question about dispatcher semantics that applies equally to events carrying no 
custom info.
   
   ## Impact and risks
   
   No behavior change for existing deployments and no configuration change. 
Audit output changes only when a layer actually stashes facts. In that case 
`customInfo()` carries those facts beside headers; the record's shape, count, 
and every other field are unchanged. Facts never appear on `httpHeaders()`.
   
   The threading contract is the same one #12692 already accepted: the 
contributor and the dispatcher must run on the same thread. That holds for the 
servlet request path.
   
   ## Reviewer focus
   
   The attach path is the thing I most want a read on. 
`IcebergTableEventDispatcher` takes the stash after the inner call returns or 
throws, then copies extras onto a new `IcebergRequestContext` so the event's 
`customInfo()` can merge them without mutating the caller's headers.
   
   Second, the empty-extras identity: `customInfo()` returns the same headers 
map it does today when nothing was stashed. If you would rather always copy, 
say so.
   
   ## Related work
   
   Fixes #12722
   
   Counterpart of #12692 for Iceberg REST table events.
   
   Nevin


-- 
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]

Reply via email to