Uh oh!
There was an error while loading. Please reload this page.
Add per-attempt retry metadata tags in OrchestrationContext.ScheduleWithRetry - #1367
Add per-attempt retry metadata tags in OrchestrationContext.ScheduleWithRetry#1367Varshitha Bachu (bachuv) wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds per-attempt retry metadata to OrchestrationContext.ScheduleWithRetry<T> by stamping reserved dt.retry.* tags onto each scheduled activity attempt, enabling downstream visibility tooling to identify retry attempt number and policy ceiling.
Changes:
- Inject
dt.retry.attemptanddt.retry.maxAttemptstags into eachScheduleWithRetryactivity scheduling attempt (formatted withInvariantCulture). - Add
RetryTagsconstants to centralize the reserved tag keys. - Add unit tests intended to validate tag emission and behavior across various retry scenarios.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/DurableTask.Core/OrchestrationContext.cs | Adds per-attempt tag injection inside ScheduleWithRetry<T> via ScheduleTaskOptions.Tags. |
src/DurableTask.Core/RetryTags.cs | Introduces reserved retry tag key constants (dt.retry.*). |
Test/DurableTask.Core.Tests/ScheduleWithRetryTagsTests.cs | Adds tests validating retry tag emission (but currently placed under Test/, which is not part of the active test projects). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| /// <summary> | ||
| /// Reserved tag keys written by DTFx into <see cref="ScheduleTaskOptions.Tags"/> (and from there into | ||
| /// <see cref="History.TaskScheduledEvent.Tags"/>) when an activity is scheduled under a retry policy. | ||
| /// </summary> |
| /// Per-attempt retry metadata is injected into the scheduled task's <see cref="ScheduleTaskOptions.Tags"/> | ||
| /// under the reserved <c>dt.retry.*</c> tag namespace (<c>dt.retry.attempt</c>, <c>dt.retry.maxAttempts</c>). | ||
| /// These tags flow through <c>ScheduleTaskOrchestratorAction.Tags</c> into the persisted | ||
| /// <c>TaskScheduledEvent.Tags</c>, where consumers (Functions extension, DTS Dashboard) can read them. |
There was a problem hiding this comment.
Can we remove this part of the comment?
/// where consumers (Functions extension, DTS Dashboard) can read them.
/// Backends that do not roundtrip <c>TaskScheduledEvent.Tags</c>
/// simply drop the tags at persistence; consumers fall back to a "metadata unavailable" path with no error.
I feel like it's leaking implementation details/we probably shouldn't be referencing Functions and DTS from here
| /// <c>TaskScheduledEvent.Tags</c>, where consumers (Functions extension, DTS Dashboard) can read them. | ||
| /// Backends that do not roundtrip <c>TaskScheduledEvent.Tags</c> | ||
| /// simply drop the tags at persistence; consumers fall back to a "metadata unavailable" path with no error. | ||
| /// The attempt counter is closure-local, so each call site builds its own counter; deterministic replay |
There was a problem hiding this comment.
Just double-checking, I'm assuming we confirmed this is actually true in testing (like a "live test", I see that we did test it in the unit tests)?
"deterministic replay reproduces the same per-attempt values because invokes the closure the same number of times in the same order on replay."
Uh oh!
There was an error while loading. Please reload this page.
| } | ||
| // --------------------------------------------------------------------- | ||
| // Core test #8: MaxNumberOfAttempts == 0 — closure is never invoked. |
There was a problem hiding this comment.
So if I understand this test correctly, we should never invoke the Activity in this case, right? I haven't looked at the other PRs yet so forgive me if we're already doing this but I wonder if we should add a check preventing a 0 value here since it seems non-sensical in that case...
sophiatev
commented
Jun 30, 2026
One thing I just realized - will this work with rewind? I think it should be fine since we just delete the |
Summary
OrchestrationContext.ScheduleWithRetry<T>now stamps two reserved tags onevery per-attempt schedule:
dt.retry.attemptInvariantCulturedt.retry.maxAttemptsretryOptions.MaxNumberOfAttempts, same formatTags flow through the existing
ScheduleTaskOptions.Tags→ScheduleTaskOrchestratorAction.Tags→persisted
TaskScheduledEvent.Tagspipeline. No dispatcher change.This is the foundational change for the native activity retry visibility
feature.
Design
Counter is scoped to a single
ScheduleWithRetrycall (local variablecaptured by the nested
RetryCallfunction):RetryInterceptor.Invokere-walks history the sameway on replay, producing identical tag values.
ScheduleTask<T>call passesoptions.RetryOptions == null, taking the non-retry path.dt.*tag prefix reserved for DTFx-injected metadata (no existing code ordocs use it).
Files
src/DurableTask.Core/OrchestrationContext.cs—ScheduleWithRetryclosure builds
ScheduleTaskOptionswith the two tags.src/DurableTask.Core/RetryTags.cs(new) — internal constants.test/DurableTask.Core.Tests/ScheduleWithRetryTagsTests.cs(new, 360 lines)— covers monotonic counter starting at 1, per-call-site independence, replay
determinism,
Handle()short-circuit,InvariantCultureformatting,multi-digit attempts,
maxAttempts=1.Compatibility
TaskScheduledEvent.Tags(Azure Storage, MSSQL,Netherite) silently drop them at persistence.