Use callback-reported close time in CHASM Nexus completion events#10915
Open
tekkaya wants to merge 2 commits into
Open
Use callback-reported close time in CHASM Nexus completion events#10915tekkaya wants to merge 2 commits into
tekkaya wants to merge 2 commits into
Conversation
Async Nexus completion callbacks carry a CloseTime — the time the operation actually completed, which may precede the moment the callback is processed by the history service. Previously the CHASM completion path dropped it and stamped the NexusOperationCompleted/Failed/Canceled events (and the operation's ClosedTime) with server-processing time. Thread the reported close time from HandleNexusCompletion through onCompleted/onFailed/onCanceled into the terminal transitions and the workflow store's history events, mirroring how EventStarted already overrides StartTime. The value is applied only when present; the sync invocation path and completions without a close time fall back to the current component time as before.
Removed comments regarding close time handling in tests.
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
1 participant
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.
What changed?
CHASM Nexus operations now stamp their terminal history events (
NexusOperationCompleted/Failed/Canceled) and the operation'sClosedTimewith the close time reported in the async completion callback, instead of the server time at which the callback happens to be processed.HandleNexusCompletionreadscompletion.GetCloseTime()and threads it throughonCompleted/onFailed/onCanceledinto the terminal transitions.OperationStoreinterface's completion methods take acloseTime *time.Time, and the workflow store overrides the emitted history event'sEventTimewhen it is set — mirroring how the started event already overridesStartTime.nil, and completions with no close time fall back to the current component time, preserving existing behavior.Why?
The completion callback's close time is the moment the operation actually finished, which can precede callback delivery/processing (queueing, retries, cross-tree fallback, reset re-apply). Recording processing time made event timing drift from reality. This matches the precedent already set for the started event's
StartTime.How did you test it?
Potential risks
Behaviour is unchanged when no close time is present (nil fallback to current time). A caller-reported close time earlier than the operation's scheduled time would make the schedule-to-close latency metric negative, but that reflects real reported timing .