nevzheng commented on PR #12692:
URL: https://github.com/apache/gravitino/pull/12692#issuecomment-5446793101
Adding a sequence diagram, since the gap this fills is easier to see than to
describe.
Both halves use the same participants and the same pipeline. Solid arrows
are calls with their payload shown; dashed arrows are return values.
```mermaid
sequenceDiagram
autonumber
participant IN as Inner dispatcher layer<br/>(policy / validation)
participant OD as TableEventDispatcher<br/>(outer, builds the event)
participant RC as RequestContext<br/>(thread-local)
participant EB as EventBus
participant AL as AuditLog + formatter
Note over IN,AL: solid arrow = call, payload shown · dashed arrow =
return value<br/>Participants and pipeline are identical in both halves
rect rgb(253, 235, 235)
Note over IN,AL: WITHOUT THIS CHANGE — the fact has no way out of the
inner layer
OD->>IN: createTable(ident, columns, properties, ...)
Note over IN: derives an audit-worthy fact,<br/>e.g. {"policy.decision":
"COMPLIANT"}
IN-->>OD: Table
Note over IN,OD: return type is Table — a single slot.<br/>the fact is
dropped at this boundary
Note over OD: builds CreateTableEvent(user, ident, TableInfo)<br/>no
argument exists to carry the fact
OD->>EB: dispatchEvent(CreateTableEvent)
EB->>AL: onPostEvent(CreateTableEvent)
Note over AL: writes 1 record<br/>"customInfo": {} ← field exists,
always empty
end
rect rgb(233, 246, 236)
Note over IN,AL: WITH THIS CHANGE — the fact travels alongside the return
OD->>IN: createTable(ident, columns, properties, ...)
Note over IN: derives the same fact
IN->>RC: setAuditExtras({"policy.decision": "COMPLIANT"})
IN-->>OD: Table
OD->>RC: takeAuditExtras()
RC-->>OD: {"policy.decision": "COMPLIANT"}<br/>and clears the slot
Note over OD: builds CreateTableEvent(user, ident,
TableInfo,<br/>{"policy.decision": "COMPLIANT"})
OD->>EB: dispatchEvent(CreateTableEvent)
EB->>AL: onPostEvent(CreateTableEvent)
Note over AL: writes 1 record<br/>"customInfo": {"policy.decision":
"COMPLIANT"}
end
```
Three things the diagram makes concrete:
- **The audit pipeline is not new and is not modified.** Everything from
`dispatchEvent` rightward is identical in both halves, which is why no listener
or formatter needed changing. `customInfo` is already a serialized field on the
record today — for table events it is simply always empty.
- **The gap is a scoping problem, not a capability one.**
`TableDispatcher.createTable` returns `Table`, one slot, so when control
returns to the outer dispatcher the inner layer's fact was never carried back.
Since the event is constructed at that point, there is nothing in scope to
populate `customInfo` with.
- **Still one event per operation.** The change enriches the existing
terminal event rather than publishing a sibling that consumers would have to
correlate.
The `takeAuditExtras()` read also clears the slot, which is what prevents a
stashed fact from leaking onto the next operation that reuses the same pooled
thread. `TestRequestContext` and `TestTableEventDispatcher` both pin that.
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]
