Signal links#2889
Conversation
| Link startWorkflowResponseLink; | ||
| // Links extracted from the inbound Nexus task. Stored once at the task-handler boundary so the | ||
| // workflow client (signal, signalWithStart) can attach them to outgoing requests via | ||
| // SignalWorkflowExecutionRequest.links, matching the Go SDK's NexusOperationLinksKey ctx value. |
There was a problem hiding this comment.
IMO don't mention the Go SDK here
| // SignalWorkflowExecutionRequest.links, matching the Go SDK's NexusOperationLinksKey ctx value. | |
| // SignalWorkflowExecutionRequest.links. |
| /** | ||
| * Set the {@code common.v1.Link}s extracted from the inbound Nexus task so they can be attached | ||
| * to any signal RPCs issued by the operation handler. | ||
| */ |
There was a problem hiding this comment.
This is for signal RPCs currently, but will be other RPCs in the near-ish™️ future right?
| public void addSignalWorkflowResponseLink(Link link) { | ||
| if (link != null) { | ||
| this.signalWorkflowResponseLinks.add(link); | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm wondering if this is too operation specific. As more operations include links, we'll keep adding new getters/setters here and then have to ensure they're all consumed when building the StartOperationResponse. A perhaps contrived example would be a nexus operation that signals a workflow, updates a workflow, then starts a workflow would have backlinks for all of those if my understanding is correct.
| // LinkConverter only returns a WorkflowEvent-shaped common.v1.Link; nexus links of | ||
| // other shapes (e.g. non-temporal URLs) come back null and are intentionally not | ||
| // forwarded onto SignalWorkflowExecutionRequest.links, which requires the | ||
| // WorkflowEvent variant. Log so a debugging session can see what was dropped. | ||
| io.temporal.api.common.v1.Link commonLink = | ||
| LinkConverter.nexusLinkToWorkflowEvent(link); | ||
| if (commonLink != null) { | ||
| inboundCommonLinks.add(commonLink); | ||
| } else { | ||
| log.warn( | ||
| "Dropping inbound Nexus link from outbound signal propagation: type='{}'," | ||
| + " url='{}' (not a parseable temporal WorkflowEvent link)", | ||
| link.getType(), | ||
| link.getUrl()); | ||
| } |
There was a problem hiding this comment.
This seems incorrect to me. A SANO will have a link to a Nexus operation populated and it should be sent to signal (and other) RPCs. I think we'll need to include a more generic helper on the LinkConverter that converts a Nexus link based on the type field to the appropriate Temporal proto link type.
| // If signal/signalWithStart RPCs issued by the handler returned backlinks, propagate | ||
| // them to the caller so the caller workflow's history event links to each signal event | ||
| // on the callee. Same set of backlinks applies to both sync and async response variants. | ||
| List<io.temporal.api.nexus.v1.Link> signalBacklinks = new ArrayList<>(); | ||
| for (io.temporal.api.common.v1.Link signalResponseLink : | ||
| CurrentNexusOperationContext.get().getSignalWorkflowResponseLinks()) { | ||
| if (!signalResponseLink.hasWorkflowEvent()) { | ||
| continue; | ||
| } | ||
| io.temporal.api.nexus.v1.Link converted = | ||
| LinkConverter.workflowEventToNexusLink(signalResponseLink.getWorkflowEvent()); | ||
| if (converted != null) { | ||
| signalBacklinks.add(converted); | ||
| } | ||
| } |
There was a problem hiding this comment.
Similar to my consideration of RPC specific accessors above, we'd need to check every link accessor do processing here. I think it'd be cleaner here as well to avoid details of the specific RPC that the handler may have invoked.
2 participants
Sending signals through Nexus now populates a link from the sender to the receiver as well as one from the receiver back to the sender.
What was changed
Why?
Checklist
Closes
How was this tested: