Populate resource_id fields for request header routing by tconley1428 · Pull Request #2226 · temporalio/sdk-go

tconley1428 · 2026-03-11T20:22:47Z

Summary

Implement logic to populate resource_id fields in request messages to support multi-cell request routing based on resource IDs.

This PR addresses the need to populate the newly added resource_id fields in various request messages so that the proxy can extract them and create appropriate temporal-resource-id headers for routing.

Changes

Workflow Task Requests

RespondWorkflowTaskCompletedRequest.resource_id → populated with workflow ID from task context
RespondWorkflowTaskFailedRequest.resource_id → populated with workflow ID from task context

Activity Task Requests

RespondActivityTaskCompletedRequest.resource_id → workflow ID for workflow activities, activity ID for standalone
RespondActivityTaskCompletedByIdRequest.resource_id → workflow ID for workflow activities, activity ID for standalone
RespondActivityTaskFailedRequest.resource_id → workflow ID for workflow activities, activity ID for standalone
RespondActivityTaskFailedByIdRequest.resource_id → workflow ID for workflow activities, activity ID for standalone
RespondActivityTaskCanceledRequest.resource_id → workflow ID for workflow activities, activity ID for standalone
RespondActivityTaskCanceledByIdRequest.resource_id → workflow ID for workflow activities, activity ID for standalone

Activity Heartbeat Requests

RecordActivityTaskHeartbeatRequest.resource_id → extracted from activity context
RecordActivityTaskHeartbeatByIdRequest.resource_id → workflow ID for workflow activities, activity ID for standalone

Batch Operation Requests

ExecuteMultiOperationRequest.resource_id → workflow ID from the start operation

Worker Requests

RecordWorkerHeartbeatRequest.resource_id → worker grouping key from client

Implementation Details

Helper Functions Added

getActivityResourceId(workflowId, activityId): Determines appropriate resource ID (workflow ID if present, otherwise activity ID)
getActivityResourceIdFromCtx(ctx): Extracts resource ID from activity context using activityEnvironment

Logic

Workflow activities: Use the workflow ID as resource_id
Standalone activities: Use the activity ID as resource_id
Multi-operation requests: Use the workflow ID from the first start operation
Worker requests: Use the client's workerGroupingKey

Test plan

Project builds successfully
TestTemporalHeaderInterceptor passes (validates header extraction works)
Activity heartbeat tests continue to pass
All resource_id fields are properly populated when requests are made

Notes

RespondQueryTaskCompletedRequest does not have a resource_id field
FetchWorkerConfigRequest and UpdateWorkerConfigRequest resource_id fields were not implemented as these APIs are not yet implemented in the SDK
Nexus requests were skipped per discussion
The existing TestTemporalHeaderInterceptor validates that the temporal-resource-id header is correctly set to "workflow:test-workflow-id", confirming the entire pipeline works

🤖 Generated with Claude Code

_**READ BEFORE MERGING:** All PRs require approval by both Server AND SDK teams before merging! This is why the number of required approvals is "2" and not "1"--two reviewers from the same team is NOT sufficient. If your PR is not approved by someone in BOTH teams, it may be summarily reverted._  Adding a proto annotation to be used for automatically propagating message fields into headers. Will be accompanied by SDK changes to generate code for doing so based on the annotations. See temporalio/api-go#236 and temporalio/sdk-go#2226 for example. Also adds additional `resource-id` fields to a number of messages for use in routing.  MCN support  **Breaking changes**  **Server PR** --------- Co-authored-by: Claude <noreply@anthropic.com>

_**READ BEFORE MERGING:** All PRs require approval by both Server AND SDK teams before merging! This is why the number of required approvals is "2" and not "1"--two reviewers from the same team is NOT sufficient. If your PR is not approved by someone in BOTH teams, it may be summarily reverted._  Adding a proto annotation to be used for automatically propagating message fields into headers. Will be accompanied by SDK changes to generate code for doing so based on the annotations. See #236 and temporalio/sdk-go#2226 for example. Also adds additional `resource-id` fields to a number of messages for use in routing.  MCN support  **Breaking changes**  **Server PR** --------- Co-authored-by: Claude <noreply@anthropic.com>

_**READ BEFORE MERGING:** All PRs require approval by both Server AND SDK teams before merging! This is why the number of required approvals is "2" and not "1"--two reviewers from the same team is NOT sufficient. If your PR is not approved by someone in BOTH teams, it may be summarily reverted._  Adding a proto annotation to be used for automatically propagating message fields into headers. Will be accompanied by SDK changes to generate code for doing so based on the annotations. See temporalio/api-go#236 and temporalio/sdk-go#2226 for example. Also adds additional `resource-id` fields to a number of messages for use in routing.  MCN support  **Breaking changes**  **Server PR** --------- Co-authored-by: Claude <noreply@anthropic.com>

tconley1428 · 2026-03-25T19:42:02Z

@claude review

claude

The implementation looks systematic and has solid test coverage, but the scope is large enough (core task handlers, pollers, grpc interceptor refactor) that a human reviewer should sign off.

Extended reasoning...

Overview

This PR populates resource_id fields across multiple request types in the Temporal Go SDK to enable multi-cell request routing. It touches internal/grpc_dialer.go, internal_task_handlers.go, internal_task_pollers.go, internal_worker_heartbeat.go, internal_workflow_client.go, internal_workflow_testsuite.go, a new 825-line test file, and all module dependency files (go.temporal.io/api v1.62.5 to v1.62.6).

Security Risks

No direct security risks. The changes populate routing metadata headers, not authentication or authorization fields. The delegation to proxy.ExtractTemporalRequestHeaders in grpc_dialer.go replaces manual namespace header extraction; the new function receives ExistingMetadata to preserve the previous behavior of not overwriting existing headers, but this behavioral equivalence depends on the external library implementation which cannot be verified here.

Level of Scrutiny

This warrants human review. The PR touches several core internal components that handle task completion, activity heartbeats, and gRPC interceptors. The grpc_dialer.go change is particularly notable: it replaces a manually written interceptor with a call to an external library function (proxy.ExtractTemporalRequestHeaders), which is a behavioral change that deserves careful attention even if it is likely correct.

Other Factors

There is a resolved inline comment questioning whether workerGroupingKey (used in RecordWorkerHeartbeatRequest.ResourceId) is correct versus instanceKey. The comment is marked resolved but there is no follow-up explanation confirming which was chosen and why. A human reviewer familiar with the routing semantics should validate this choice. The CompleteActivity path passes empty strings for workflowId/activityId, resulting in an empty resource_id; this appears intentional (task-token-based completion lacks workflow context) but deserves explicit confirmation.

Implement logic to populate resource_id fields in request messages to support multi-cell request routing based on resource IDs. Changes: - Workflow task requests: Populate with workflow ID from task context - Activity task requests: Use workflow ID for workflow activities, activity ID for standalone activities - Activity heartbeat requests: Extract resource ID from activity context - Batch operation requests: Use workflow ID from start operation - Worker heartbeat requests: Use worker grouping key Added helper functions: - getActivityResourceId(workflowId, activityId): Determines appropriate resource ID - getActivityResourceIdFromCtx(ctx): Extracts resource ID from activity context All resource_id fields are now properly populated to enable correct header extraction and routing via the temporal-resource-id header. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Implement resource_id field population for Nexus task completion and failure requests. Changes: - Modified fillInCompletion() to accept resourceId parameter and populate RespondNexusTaskCompletedRequest.resource_id - Modified fillInFailure() to accept resourceId parameter and populate RespondNexusTaskFailedRequest.resource_id - Updated all calls to pass task.ResourceId from PollNexusTaskQueueResponse The resource_id field is now populated with the value from PollNexusTaskQueueResponse.resource_id as specified in the proto comments, enabling proper request routing via temporal-resource-id headers. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

This commit adds tests for all 8 activity task resource ID fields to ensure proper resource_id population for multi-cell routing. Key additions: - Comprehensive test suite in resource_id_impl_test.go covering: * All 3 workflow task requests (completed, failed, query) * All 8 activity task requests using conversion function validation * Both workflow and standalone activity scenarios - Uses convertActivityResultToRespondRequest and convertActivityResultToRespondRequestByID as validation points to test resource_id field population - Tests cover RespondActivityTaskCompleted/Failed/Canceled and ByID variants - Updated RESOURCE_ID_FIELDS.md with detailed testing status (10/15 tested, 66.7% coverage) All tests pass and validate that: - Workflow ID is used when present - Activity ID is used for standalone activities (empty workflow ID) - Conversion functions properly populate resource_id fields via getActivityResourceId helper 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Add comprehensive test for RecordActivityTaskHeartbeatByIdRequest resource_id field to move from partial to full test coverage. Changes: - Added testActivityTaskHeartbeatByIdResourceID function with proper mocking - Tests both workflow execution and standalone activity scenarios - Validates resource_id field is set to workflow ID when present, activity ID when standalone - Uses RecordActivityHeartbeatByID client method with proper service mocking - Updated RESOURCE_ID_FIELDS.md to reflect full test coverage (11/15 tested, 73.3%) This completes testing for all 8 activity task resource ID fields, bringing activity task test coverage to 100% (8/8 tested). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Remove resource_id field implementations and tests for messages that no longer have resource_id fields in the proto definitions: - RespondQueryTaskCompletedRequest: Removed ResourceId assignment and test - RespondNexusTaskCompletedRequest: Removed ResourceId field and unused resourceId parameter - RespondNexusTaskFailedRequest: Removed ResourceId field and unused resourceId parameter Also add comprehensive test for ExecuteMultiOperationRequest resource_id field, bringing test coverage to 91.7% (11/12 implemented fields tested). Changes: - Remove ResourceId assignments in task handlers and pollers - Remove unused resourceId parameters from fillInCompletion() and fillInFailure() - Add testExecuteMultiOperationResourceID() with UpdateWithStartWorkflow validation - Update RESOURCE_ID_FIELDS.md to reflect current proto state and test coverage 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Verified that all 12 resource ID fields have proper test coverage by: - Temporarily removing ResourceId assignments - Confirming tests fail with empty ResourceId - Restoring ResourceId assignments - Verifying tests pass with ResourceId restored This ensures all resource ID tests will catch regressions where fields are not populated correctly during request routing. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Rename resource_id_impl_test.go to resource_id_test.go for conventional naming - Remove RESOURCE_ID_FIELDS.md working state file 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- All resource_id fields now carry fully-prefixed values from the SDK (e.g., "workflow:id", "worker:key", "activity:id") - Consolidate getActivityResourceIdFromCtx to delegate to getActivityResourceId instead of duplicating prefix logic - Return empty string from getActivityResourceId when both IDs are empty - Update tests to expect prefixed values Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Upgrade to published api-go v1.62.6 which includes the request header proxy generator and standardized resource_id annotations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Remove unused createTestQueryTask helper and querypb import - Add TestGetActivityResourceId_BothEmpty for the edge case where both workflowId and activityId are empty Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Pass opts.WorkflowID to convertActivityResultToRespondRequest in CompleteActivityWithOptions so async completions include the temporal-resource-id header for proxy routing. ActivityID is not available on this code path (task-token based identification). - Use nil-safe getter GetWorkflowId() instead of direct field access on task.WorkflowExecution in completeWorkflow, consistent with all other usages in this file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

yuandrew

LGTM, once we retarget this PR to feature-mcn, good to approve and merge

…rce-id-fields # Conflicts: # internal/grpc_dialer.go

* Automatic request header support * Populate resource_id fields for request header routing Implement logic to populate resource_id fields in request messages to support multi-cell request routing based on resource IDs. Changes: - Workflow task requests: Populate with workflow ID from task context - Activity task requests: Use workflow ID for workflow activities, activity ID for standalone activities - Activity heartbeat requests: Extract resource ID from activity context - Batch operation requests: Use workflow ID from start operation - Worker heartbeat requests: Use worker grouping key Added helper functions: - getActivityResourceId(workflowId, activityId): Determines appropriate resource ID - getActivityResourceIdFromCtx(ctx): Extracts resource ID from activity context All resource_id fields are now properly populated to enable correct header extraction and routing via the temporal-resource-id header. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Add resource_id population for Nexus task requests Implement resource_id field population for Nexus task completion and failure requests. Changes: - Modified fillInCompletion() to accept resourceId parameter and populate RespondNexusTaskCompletedRequest.resource_id - Modified fillInFailure() to accept resourceId parameter and populate RespondNexusTaskFailedRequest.resource_id - Updated all calls to pass task.ResourceId from PollNexusTaskQueueResponse The resource_id field is now populated with the value from PollNexusTaskQueueResponse.resource_id as specified in the proto comments, enabling proper request routing via temporal-resource-id headers. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Add comprehensive activity task resource ID tests This commit adds tests for all 8 activity task resource ID fields to ensure proper resource_id population for multi-cell routing. Key additions: - Comprehensive test suite in resource_id_impl_test.go covering: * All 3 workflow task requests (completed, failed, query) * All 8 activity task requests using conversion function validation * Both workflow and standalone activity scenarios - Uses convertActivityResultToRespondRequest and convertActivityResultToRespondRequestByID as validation points to test resource_id field population - Tests cover RespondActivityTaskCompleted/Failed/Canceled and ByID variants - Updated RESOURCE_ID_FIELDS.md with detailed testing status (10/15 tested, 66.7% coverage) All tests pass and validate that: - Workflow ID is used when present - Activity ID is used for standalone activities (empty workflow ID) - Conversion functions properly populate resource_id fields via getActivityResourceId helper 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix partial validation of RecordActivityTaskHeartbeatByIdRequest Add comprehensive test for RecordActivityTaskHeartbeatByIdRequest resource_id field to move from partial to full test coverage. Changes: - Added testActivityTaskHeartbeatByIdResourceID function with proper mocking - Tests both workflow execution and standalone activity scenarios - Validates resource_id field is set to workflow ID when present, activity ID when standalone - Uses RecordActivityHeartbeatByID client method with proper service mocking - Updated RESOURCE_ID_FIELDS.md to reflect full test coverage (11/15 tested, 73.3%) This completes testing for all 8 activity task resource ID fields, bringing activity task test coverage to 100% (8/8 tested). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Remove resource_id implementations for messages without proto fields Remove resource_id field implementations and tests for messages that no longer have resource_id fields in the proto definitions: - RespondQueryTaskCompletedRequest: Removed ResourceId assignment and test - RespondNexusTaskCompletedRequest: Removed ResourceId field and unused resourceId parameter - RespondNexusTaskFailedRequest: Removed ResourceId field and unused resourceId parameter Also add comprehensive test for ExecuteMultiOperationRequest resource_id field, bringing test coverage to 91.7% (11/12 implemented fields tested). Changes: - Remove ResourceId assignments in task handlers and pollers - Remove unused resourceId parameters from fillInCompletion() and fillInFailure() - Add testExecuteMultiOperationResourceID() with UpdateWithStartWorkflow validation - Update RESOURCE_ID_FIELDS.md to reflect current proto state and test coverage 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Complete systematic verification of all resource ID field tests Verified that all 12 resource ID fields have proper test coverage by: - Temporarily removing ResourceId assignments - Confirming tests fail with empty ResourceId - Restoring ResourceId assignments - Verifying tests pass with ResourceId restored This ensures all resource ID tests will catch regressions where fields are not populated correctly during request routing. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Rename test file and remove working state documentation - Rename resource_id_impl_test.go to resource_id_test.go for conventional naming - Remove RESOURCE_ID_FIELDS.md working state file 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Update resource ids with prefixes * Standardize resource_id prefix responsibility in SDK - All resource_id fields now carry fully-prefixed values from the SDK (e.g., "workflow:id", "worker:key", "activity:id") - Consolidate getActivityResourceIdFromCtx to delegate to getActivityResourceId instead of duplicating prefix logic - Return empty string from getActivityResourceId when both IDs are empty - Update tests to expect prefixed values Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update go.temporal.io/api to v1.62.6, remove local replace directive Upgrade to published api-go v1.62.6 which includes the request header proxy generator and standardized resource_id annotations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove unused test helper and add empty resource ID test - Remove unused createTestQueryTask helper and querypb import - Add TestGetActivityResourceId_BothEmpty for the edge case where both workflowId and activityId are empty Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review feedback: fix CompleteActivity resource ID and nil-safety - Pass opts.WorkflowID to convertActivityResultToRespondRequest in CompleteActivityWithOptions so async completions include the temporal-resource-id header for proxy routing. ActivityID is not available on this code path (task-token based identification). - Use nil-safe getter GetWorkflowId() instead of direct field access on task.WorkflowExecution in completeWorkflow, consistent with all other usages in this file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address resource ID review feedback --------- Co-authored-by: Chad Retz <chad@temporal.io> Co-authored-by: Claude <noreply@anthropic.com>

tconley1428 requested a review from a team as a code owner March 11, 2026 20:22

tconley1428 marked this pull request as draft March 11, 2026 20:24

tconley1428 mentioned this pull request Mar 11, 2026

Add request header annotation and resource-id fields temporalio/api#728

Merged

tconley1428 commented Mar 11, 2026

View reviewed changes

Comment thread internal/internal_worker_heartbeat.go Outdated

tconley1428 mentioned this pull request Mar 24, 2026

Automatic request header code generation for resource routing temporalio/sdk-rust#1179

Open

tconley1428 marked this pull request as ready for review March 25, 2026 19:42

claude Bot reviewed Mar 25, 2026

View reviewed changes

claude Bot reviewed Mar 26, 2026

View reviewed changes

Comment thread internal/internal_workflow_client.go Outdated

Comment thread internal/internal_task_handlers.go Outdated

claude Bot reviewed Mar 26, 2026

View reviewed changes

Comment thread internal/internal_task_handlers.go Outdated

Comment thread internal/internal_task_pollers.go Outdated

yuandrew reviewed Mar 31, 2026

View reviewed changes

cretz and others added 13 commits June 16, 2026 08:07

Automatic request header support

3db84e4

Update resource ids with prefixes

fcb1960

Update go.temporal.io/api to v1.62.6, remove local replace directive

7361a7e

Upgrade to published api-go v1.62.6 which includes the request header proxy generator and standardized resource_id annotations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

tconley1428 force-pushed the populate-resource-id-fields branch from 7df41ba to 0bc3e31 Compare June 16, 2026 15:19

tconley1428 and others added 2 commits June 16, 2026 08:23

Address resource ID review feedback

710ce96

Merge branch 'main' into populate-resource-id-fields

bb9d42d

tconley1428 added 2 commits June 23, 2026 15:36

Merge branch 'main' into populate-resource-id-fields

8970d29

Merge branch 'main' into populate-resource-id-fields

3b10c2d

yuandrew reviewed Jul 1, 2026

View reviewed changes

tconley1428 changed the base branch from main to feature-mcn July 9, 2026 16:28

Merge remote-tracking branch 'origin/feature-mcn' into populate-resou…

428d5a4

…rce-id-fields # Conflicts: # internal/grpc_dialer.go

tconley1428 merged commit 94eddda into feature-mcn Jul 9, 2026
33 of 37 checks passed

tconley1428 deleted the populate-resource-id-fields branch July 9, 2026 16:33

Navigation Menu

Uh oh!

Populate resource_id fields for request header routing - #2226

Populate resource_id fields for request header routing#2226
tconley1428 merged 18 commits into
feature-mcnfrom
populate-resource-id-fields

tconley1428 commented Mar 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

tconley1428 commented Mar 25, 2026

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yuandrew left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

tconley1428 commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Workflow Task Requests

Activity Task Requests

Activity Heartbeat Requests

Batch Operation Requests

Worker Requests

Implementation Details

Helper Functions Added

Logic

Test plan

Notes

Uh oh!

Uh oh!

tconley1428 commented Mar 25, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security Risks

Level of Scrutiny

Other Factors

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yuandrew left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tconley1428 commented Mar 11, 2026 •

edited

Loading