[api][runtime][python] Add minimal Event lineage - #923
Conversation
|
One ActionStateStore replay semantic seems worth making explicit before merging. When a completed action result is reused, the runtime returns the stored output Events and rebinds their lineage to the current triggering Event while preserving the output Event IDs. The resulting behavior is: I think this is a reasonable semantic, but it is currently implicit. Could we document it and add a test that asserts |
3caaad9 to
7d122a1
Compare
7d122a1 to
d8432ea
Compare
| """Render valid Trace Trees without assigning meaning to sibling order.""" | ||
| lines: list[str] = [] | ||
|
|
||
| def render_event(event_id: str, indent: str) -> None: |
There was a problem hiding this comment.
The reader intentionally accepts the same Event ID with multiple lineage edges, but that also means the resulting graph is not guaranteed to be acyclic. For example, observations root -> A, A -> B, and B -> A pass the current validation. render_event has no active-path or edge guard, so text rendering keeps pushing A and B indefinitely. This is broader than the direct self-loop case raised earlier. Could the reader detect a cyclic edge (or track the active render path), emit a warning, and stop only that branch while retaining valid DAG reuse?
| """ | ||
|
|
||
| id: UUID = Field(default=None) | ||
| id: UUID = Field(default_factory=uuid4, frozen=True) |
There was a problem hiding this comment.
frozen=True makes Event.id immutable, so direct reassignment now raises a ValidationError. On current main, the built-in from_event methods and workflow documentation use result.id = event.id; this PR migrates those call sites, but external custom Event subclasses following the existing guidance will break. Since immutability is a user-visible compatibility change separate from switching ID generation to UUIDv4, could it be called out explicitly in the PR description or a migration note, with reconstruct_from() as the replacement?
| f"across {len(matching_records)} records.", | ||
| ) | ||
| ) | ||
| continue |
There was a problem hiding this comment.
continue skips building a node for the conflicting id, and every descendant leaves the rendered trees with it, not just the conflicting Event.
Six records in (a root, K logged twice with different content, then C <- K, D <- C, E <- D):
warnings: [('EVENT_ID_CONFLICT', 'k'), ('MISSING_PARENT', 'c')]
nodes: ['c', 'd', 'e', 'r']
text:
Trace Tree 1
_input_event (r)
C gets a MISSING_PARENT, but D and E get nothing, since they link cleanly to nodes that are themselves unreachable. Four Events drop out of the rendered tree. They stay in nodes, but nothing links them to a root, and two warnings name two of them.
The path I can see reaching this: FileEventLogger truncates at write time from event-log.standard.max-string-length and friends, so a reused Event ID logged before and after a threshold change differs in content and reads as a conflict. That is the cross-restart case the new hint block describes.
Could the conflicting id keep its first observation as the node, so the branch survives and the inconsistency still surfaces as a warning?
Linked issue: Closes #841
Related discussions:
Purpose of change
The Event Log currently records individual Events but does not preserve enough information to reconstruct the direct causal path between an Event, the Action it triggered, and the Events emitted by that Action.
This PR implements the minimal Event-lineage phase discussed in #710.
It adds two framework-managed fields to each Event emitted by an Action:
upstreamEventId: the ID of the Event consumed by the emitting Action;upstreamActionName: the name of the Action that emitted the current Event.Together with the existing per-occurrence Event ID, these fields are sufficient to reconstruct an InputEvent-rooted Event-Action-Event causal tree from the available Event Log records.
This PR deliberately does not implement the complete execution-tracing model proposed in #900. Run identity, concrete Action invocation identity, execution hierarchy, lifecycle, retry/replay semantics, ordering, and failure details remain part of that separate design.
A real Runtime execution was verified with the following causal path:
The File Event Log contained three physical Event records:
upstreamEventIdupstreamActionName_input_eventMiddleEventaction1_output_eventaction2Running
tools/reconstruct_trace_tree.pyagainst that Runtime log produced:The JSON reconstruction contained one root, all three Event nodes, the two expected virtual Action edges, and no warnings.
Tests
Add e2e test to review Trace Tree
python/flink_agents/e2e_tests/e2e_tests_integration/python_event_logging_test.pyAPI
This PR changes the default Python Event ID generation strategy from content-derived IDs to UUIDv4.
This is also a breaking Python API change:
Event.idis now immutable. CustomEvent.from_event()implementations that useresult.id = event.idmust instead returnresult.reconstruct_from(event).Documentation
doc-neededdoc-not-neededdoc-included