Populate resource_id fields for request header routing#2226
Draft
tconley1428 wants to merge 10 commits intomasterfrom
Draft
Populate resource_id fields for request header routing#2226tconley1428 wants to merge 10 commits intomasterfrom
tconley1428 wants to merge 10 commits intomasterfrom
Conversation
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>
tconley1428
commented
Mar 11, 2026
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>
tconley1428
added a commit
to temporalio/api
that referenced
this pull request
Mar 16, 2026
_**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._ <!-- Describe what has changed in this PR --> 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. <!-- Tell your future self why have you made these changes --> MCN support <!-- Are there any breaking changes on binary or code level? --> **Breaking changes** <!-- If this breaks the Server, please provide the Server PR to merge right after this PR was merged. --> **Server PR** --------- Co-authored-by: Claude <noreply@anthropic.com>
temporal-cicd bot
pushed a commit
to temporalio/api-go
that referenced
this pull request
Mar 16, 2026
_**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._ <!-- Describe what has changed in this PR --> 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. <!-- Tell your future self why have you made these changes --> MCN support <!-- Are there any breaking changes on binary or code level? --> **Breaking changes** <!-- If this breaks the Server, please provide the Server PR to merge right after this PR was merged. --> **Server PR** --------- Co-authored-by: Claude <noreply@anthropic.com>
stephanos
pushed a commit
to temporalio/api
that referenced
this pull request
Mar 17, 2026
_**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._ <!-- Describe what has changed in this PR --> 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. <!-- Tell your future self why have you made these changes --> MCN support <!-- Are there any breaking changes on binary or code level? --> **Breaking changes** <!-- If this breaks the Server, please provide the Server PR to merge right after this PR was merged. --> **Server PR** --------- 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>
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
2 participants
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.
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 contextRespondWorkflowTaskFailedRequest.resource_id→ populated with workflow ID from task contextActivity Task Requests
RespondActivityTaskCompletedRequest.resource_id→ workflow ID for workflow activities, activity ID for standaloneRespondActivityTaskCompletedByIdRequest.resource_id→ workflow ID for workflow activities, activity ID for standaloneRespondActivityTaskFailedRequest.resource_id→ workflow ID for workflow activities, activity ID for standaloneRespondActivityTaskFailedByIdRequest.resource_id→ workflow ID for workflow activities, activity ID for standaloneRespondActivityTaskCanceledRequest.resource_id→ workflow ID for workflow activities, activity ID for standaloneRespondActivityTaskCanceledByIdRequest.resource_id→ workflow ID for workflow activities, activity ID for standaloneActivity Heartbeat Requests
RecordActivityTaskHeartbeatRequest.resource_id→ extracted from activity contextRecordActivityTaskHeartbeatByIdRequest.resource_id→ workflow ID for workflow activities, activity ID for standaloneBatch Operation Requests
ExecuteMultiOperationRequest.resource_id→ workflow ID from the start operationWorker Requests
RecordWorkerHeartbeatRequest.resource_id→ worker grouping key from clientImplementation 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 activityEnvironmentLogic
Test plan
TestTemporalHeaderInterceptorpasses (validates header extraction works)Notes
RespondQueryTaskCompletedRequestdoes not have aresource_idfieldFetchWorkerConfigRequestandUpdateWorkerConfigRequestresource_id fields were not implemented as these APIs are not yet implemented in the SDKTestTemporalHeaderInterceptorvalidates that the temporal-resource-id header is correctly set to "workflow:test-workflow-id", confirming the entire pipeline works🤖 Generated with Claude Code