Skip to content

Activity tag support#426

Merged
YunchuWang merged 11 commits into
mainfrom
wangbill/atag
Jun 10, 2025
Merged

Activity tag support#426
YunchuWang merged 11 commits into
mainfrom
wangbill/atag

Conversation

@YunchuWang

@YunchuWang YunchuWang commented May 10, 2025

Copy link
Copy Markdown
Member

This pull request introduces several enhancements and new features to improve the functionality and flexibility of the Durable Task framework. The most significant changes include the addition of tagging support for tasks, the creation of a new activity class for cache clearing, and updates to orchestrator logic to utilize the new activity. Below is a breakdown of the changes:

Related prs:
https://github.com/microsoft/durabletask-protobuf/pull/43/files
https://github.com/Azure/durabletask/pull/1211/files
https://msazure.visualstudio.com/One/_git/AAPT-DTMB/pullrequest/12388270

Task Tagging Support

  • Added a Tags property to the TaskOptions class, enabling tasks to have associated metadata tags. This includes a new constructor for initializing TaskOptions with tags. (src/Abstractions/TaskOptions.cs, src/Abstractions/TaskOptions.csR22-R42)
  • Introduced a CallActivityOptions class, inheriting from TaskOptions, specifically for configuring activity calls with tags. (src/Abstractions/TaskOptions.cs, src/Abstractions/TaskOptions.csR128-R153)
  • Updated ProtoUtils to handle tags during task scheduling and history event conversion, ensuring tags are serialized and deserialized properly. (src/Shared/Grpc/ProtoUtils.cs, [1] [2]
  • Modified TaskOrchestrationContextWrapper to pass tags when scheduling tasks, ensuring they are included in the orchestration execution. (src/Worker/Core/Shims/TaskOrchestrationContextWrapper.cs, src/Worker/Core/Shims/TaskOrchestrationContextWrapper.csR141-R170)

New Cache Clearing Activity

Integration with Sample Application

  • Registered the CacheClearingActivity and updated the Durable Task worker configuration in the sample application to include the new activity. (samples/ScheduleWebApp/Program.cs, samples/ScheduleWebApp/Program.csR23-L28)
  • Added necessary using directives for the new activity in the orchestrator and program files. (samples/ScheduleWebApp/Orchestrations/CacheClearingOrchestrator.cs, [1]; samples/ScheduleWebApp/Program.cs, [2]

Dependency Updates

  • Upgraded the Microsoft.Azure.DurableTask.Core package from version 3.1.0 to 3.1.1 to ensure compatibility with the new features. (Directory.Packages.props, Directory.Packages.propsL31-R31)

Code Cleanup

all durabletask-dotnet ci tests passed with activity tag changes.
i see the inmemoryorchservice used in integ test do not support getorchestrationhistory so will add the integ tests for this to dts repo.

I built the SDK locally with activity tag support, added the tests in DTMB, and verified that all related tests pass successfully.

image

i will merge these tests to DTMB Pull request 12627874: portable dotnet sdks version bump + add portable sdk activity tag integration... - Repos
after next portable dotnet sdks release including activity tag.

@YunchuWang YunchuWang marked this pull request as ready for review May 12, 2025 05:43

@cgillum cgillum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to revisit the public API change. In particular, I don't think we should limit tags to activity calls.

Comment thread src/Shared/Grpc/ProtoUtils.cs Outdated
Comment thread src/Shared/Grpc/ProtoUtils.cs Outdated
Comment thread src/Abstractions/TaskOptions.cs Outdated
Comment thread samples/ScheduleWebApp/Activities/CacheClearingActivity.cs Outdated
@YunchuWang YunchuWang requested a review from cgillum May 12, 2025 22:17
Comment thread src/Abstractions/TaskOptions.cs Outdated
@YunchuWang YunchuWang requested a review from cgillum May 12, 2025 22:56
@YunchuWang

Copy link
Copy Markdown
Member Author

@cgillum Hi Chris, all tests passed (tested locally works).
image i see the inmemoryorchservice used in integ test do not support getorchestrationhistory, i will add the integration test to dts after this is released.
can I get approval? thanks

@kaibocai kaibocai left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, Please get @cgillum approval as well!

@YunchuWang YunchuWang dismissed cgillum’s stale review June 9, 2025 19:17

changes updated, requesting review again

cgillum
cgillum previously requested changes Jun 9, 2025
Comment thread src/Worker/Core/Shims/TaskOrchestrationContextWrapper.cs Outdated
Comment thread src/Abstractions/TaskOptions.cs Outdated
@YunchuWang YunchuWang requested a review from cgillum June 10, 2025 00:38
@YunchuWang YunchuWang dismissed cgillum’s stale review June 10, 2025 00:58

updated, requesting review again

@nytian nytian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a integration tests, e.g. in here https://github.com/microsoft/durabletask-dotnet/blob/main/test/Grpc.IntegrationTests/OrchestrationPatterns.cs to simulate calling activity with tags? Just want to make sure this behavior is ensured, as we don't run samples in the CI usually.

Comment thread samples/ScheduleWebApp/Orchestrations/CacheClearingOrchestrator.cs
@YunchuWang YunchuWang requested a review from nytian June 10, 2025 21:15
@YunchuWang YunchuWang merged commit 9a796c2 into main Jun 10, 2025
4 checks passed
@YunchuWang YunchuWang deleted the wangbill/atag branch June 10, 2025 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants