Repository navigation
fix(otel): isolate invocations and replay telemetry - #758
Open
zhongkechen wants to merge 70 commits into
Open
zhongkechen wants to merge 70 commits into
zhongkechen wants to merge 70 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Codex AI reviewNo actionable findings. Static review covered the complete PR diff. Tests, type checks, and cloud conformance were not executed, so runtime and concurrency behavior remain unverified. Reviewed commit |
This branch was successfully deployed
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.
Callback completions could be exported again after a successful input-only resume, or lost after their completed parent span was removed. Overlapping invocations could also clear tracing state held by one shared plugin instance. This change repairs those delivery/lifetime paths when operation metadata is available and makes local replay-state reads follow the service's pruning rule.
Runtime behavior
Each handler invocation owns its plugin executor, terminal-notification accounting and OTel Context state. Visible terminal input updates are dispatched after all Start hooks. Checkpoint and asynchronous hook workers carry the selected invocation Context; Start/End ordering, failed-Start isolation, token ownership, serialization and resource cleanup remain intact.
Completed Execution-view contexts retain their actual non-recording parent context for later checkpoint deliveries. Both views retain known completed parent metadata from the full visible input snapshot. Invocation-view completions wait for a current ReplayChildren segment before using a known completed input parent at End. Missing/nonterminal parentage is reported after cleanup, and private queue draining does not re-enter a public hook or replace existing error-object identity.
Explicit FAILED, TIMED_OUT and STOPPED operations export ERROR even without diagnostic details. Exception events require actual details and use the corresponding span-end timestamp. Cancellation and successful outcomes keep their existing meaning; the public core retry enum remains unchanged while its OTel attribute is RETRYING.
Only provider
force_flushI/O runs off the lifecycle worker. Admission and completion share the existing 30-second budget, with one active provider-I/O attempt per process and no queued I/O tasks. A timed-out attempt retains its slot until actual completion; healthy callers can wait for admission within their own deadline. This does not cancel an uncooperative exporter, bound synchronous span-processor callbacks/application-owned shutdown, or guarantee remaining Lambda execution time.Testing service parity
Invocation input and GetDurableExecutionState omit an operation if any ancestor is a CONTEXT in SUCCEEDED or FAILED without
ReplayChildren: true. The walk includes STARTED intermediates, is independent of operation ordering and terminates on cycles. Roots and orphans remain visible. A descendant's own retention flag cannot override a pruning ancestor.START ignores ContextOptions.ReplayChildren. The closing update records true only when explicitly supplied; otherwise it records false. Testing service JSON includes the closing false without changing Core's public bool type or generic serializer. Checkpoint responses still contain every updated operation; UpdatedOperationIds can include hidden IDs. Raw stores, public nested operation inspection, results and history retain the full execution.
The runner acknowledges the sequence captured in an invocation's input only after a valid response, including checkpoint-free PENDING. Failed delivery retains updates, newer updates survive older acknowledgments, and legacy sequence-zero records can consume acknowledged IDs. Callback completion producers and detailed history preserve the distinction between absent runtime error details and a present empty history error payload.
An updated ID alone does not supply a pruned operation record. Earlier local cached-child input tests assumed metadata that the service omits. The input-delivery regression now uses a real large child result to retain ReplayChildren metadata: the child body replays, its saved step executes once, and both views retain strict terminal-count, parent-ID, outcome and hook-error assertions. Separate public tests prove hidden IDs remain present while input/GetState omit their records, and that later checkpoint responses can still deliver an actual completion after its ordinary parent was skipped. No input, hook or telemetry workaround manufactures missing metadata.
Conformance and validation
The existing 26 cases per view remain intact, including caught-failure case25, rich-error case17, RETRYING case24 and external-completion/replay case26. The test reference remains
05960beb6c81d7b24b53114f2581b3b6f9fef6d4; the reusable-workflow pin, CodeBuild runner, locks, queues and gates are unchanged.For current service-parity commit
4654b348e83f0cf039f07acc28f16bf6ccd723f7:7fd24a1; they pass on the candidate. They cover Memory/FS, first/resumed invocation input and GetState, close flags, nested pruning, running descendants, ordering, cycles, hidden updated IDs and complete raw inspection/history.The preceding
15ab1abbcloud run 38642166646 passed 112 checks after one targeted retry of pre-test PublishLayerVersion throttling. That is historical evidence, not the current head's CI result or qualification of the separate strengthened cross-execution ancestry scenario. Current-head CI runs independently.Limits and merge prerequisites
There is no exporter acknowledgment in durable state. Metadata can be redelivered after a failed invocation/checkpoint transport attempt without a successful response acknowledgment; this is not exactly-once telemetry across failed/redelivered invocations. Already-checkpointed step bodies remain skipped. No global exported-operation cache drops a first delivery.
Virtual FLAT-map replay duplication remains a separate open issue (JavaScript #997); the service-pruning work is analogous to JavaScript #1002. A still-running map retains its children, so this work does not establish a persisted completion boundary for virtual iterations.
Shared conformance PR #131, current-head CI, effective approval and normal repository protections remain merge prerequisites. The existing Core >=2.1.0 OTel boundary and coordinated release order remain unchanged; Python #756 is already merged. Temporary propagation diagnostic references are never-merge fixtures, not adopter pins or release artifacts. No package, tag or layer is published here.