Propagate backlinks on Signal and Signal-with-Start responses

[long-nt-tran]

What changed?

With these PRs to add the linking on the Signal and Signal-with-Start responses, This PR adds logic from the server that:

• Attaches requestID from Signal and Signal-with-Start requests to the mutable state + event store so these requestIDs stay in buffer
• Return a backlink in the response that references the requestID
• On buffer flush to the DB transaction, attach these requestID to a concrete eventID, which would allow users to later know which event correlated w/ this request

In functional tests, I augmented existing tests for Signal and Signal-with-Start to:

• Ensure that backlink is returned via the responses
• Later use DescribeWorkflow to ensure that we get a concrete EventID (mapped when buffer flushed)
• Multiple signals with the same requestID gets de-dupped

Note

Doing it this way from discussion w/ @Quinn-With-Two-Ns since signals are buffered and we will not have a concrete event ID.

Why?

This will enable the caller of the signal to have a backlink to the cross-namespace signal invoked, which will become more relevant for Nexus SDK ergonomics.

How did you test it?

• built
• run locally and tested manually
• covered by existing tests
• added new unit test(s)
• added new functional test(s)

Functional tests

$ go test ./tests/ -run TestLinksTestSuite
ok      go.temporal.io/server/tests     1.486s

$ go test ./tests/ -run 'TestNexusWorkflowTestSuite' -count=1
ok      go.temporal.io/server/tests     4.714s

Potential risks

Need to test end-to-end to see that the link shows up correctly in the Web UI.

[long-nt-tran]

only pointing to my fork for CI tests, will wait for temporalio/api#761 and subsequent api-go codegen to be merged and then remove these before I merge this PR

[bergundy]

Here's what's missing:

• Adding to the map that tracks request ID to event ID
• A functional test that verifies that request IDs for all of these requests are added to the map
• Ensure that the workflow start link is only populated if an execution was started

[long-nt-tran]

temp, will delete once api PR temporalio/api#761 merges

[long-nt-tran]

Addressed feedback, I think the cleanest way to have the requestID be in the mutable state is to have it on the event is to have it be a new field in WorkflowExecutionSignaledEventAttributes, so I did need to add this field to the api PR. Let me know if there are other preferable alternatives -- I saw some other events also do the same thing of having requestID be part of the attributes so followed that pattern.

Also put all testing at functional test level so we can end-to-end assert on request ID presence + concrete event ID resolution -- edited summary to reflect this.

cc @bergundy @Quinn-With-Two-Ns

[bergundy]

Addressed feedback, I think the cleanest way to have the requestID be in the mutable state is to have it on the event is to have it be a new field in WorkflowExecutionSignaledEventAttributes, so I did need to add this field to the api PR. Let me know if there are other preferable alternatives -- I saw some other events also do the same thing of having requestID be part of the attributes so followed that pattern.

Also put all testing at functional test level so we can end-to-end assert on request ID presence + concrete event ID resolution -- edited summary to reflect this.

cc @bergundy @Quinn-With-Two-Ns

That makes sense to me because the signal could be cherry picked into a new history branch on conflict resolution and reset and you'll want to carry over the same request ID when that happens.

[bergundy]

Namespace is empty here. Looks like the bug was pre-existing.

[long-nt-tran]

yeah i noticed that too but wasn't sure if it was expected, was planning to look into it later to not change too much, but probably ok to opportunistically fix here

[bergundy]

Not quite seeing the value of attaching the buffered event ID here. It's supposed to be a temporary ID.
Also not quite sure how the request ID mapping works with signal-with-start where the same request ID maps to two history events. We shouldn't override the request ID of the WorkflowExecutionStarted event.

I think we may need to extend the proto to allow a single request ID to reference two separate events.