Skip to content

Add timer origin to the history information#1627

Open
acroca wants to merge 7 commits into
dapr:masterfrom
acroca:timer-origins
Open

Add timer origin to the history information#1627
acroca wants to merge 7 commits into
dapr:masterfrom
acroca:timer-origins

Conversation

@acroca
Copy link
Copy Markdown
Member

@acroca acroca commented Apr 9, 2026

Timers have origin now. This PR displays the timer origin.

Signed-off-by: Albert Callarisa <albert@diagrid.io>
@acroca acroca requested a review from Copilot April 9, 2026 14:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds display of timer origin information in workflow history output (details + attrs) and validates it via unit and e2e tests.

Changes:

  • Extend pkg/workflow history formatting to surface timer origin (createTimer / externalEvent / activityRetry / childWorkflowRetry).
  • Add unit tests for origin formatting and details derivation.
  • Add e2e coverage and supporting test workflows/activities for retry-origin timers; bump related Go module deps.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
tests/e2e/standalone/workflow_test.go Adds e2e assertions that history output includes timer origin in both text details and JSON attrs.
tests/apps/workflow/app.go Registers new workflows/activities used to generate retry-origin timer events in e2e.
pkg/workflow/history.go Adds origin decoding into history “Attrs” and “Details” output.
pkg/workflow/history_test.go New unit tests for timer origin formatting and inclusion in derived details.
go.mod Bumps durabletask-go + kit and adds new indirect deps.
go.sum Updates sums for the module changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/e2e/standalone/workflow_test.go Outdated
Comment thread tests/e2e/standalone/workflow_test.go Outdated
Comment thread tests/e2e/standalone/workflow_test.go Outdated
Comment thread tests/e2e/standalone/workflow_test.go Outdated
Comment thread tests/e2e/standalone/workflow_test.go Outdated
Comment thread tests/e2e/standalone/workflow_test.go Outdated
Comment thread tests/e2e/standalone/workflow_test.go Outdated
Comment thread tests/e2e/standalone/workflow_test.go Outdated
Comment thread pkg/workflow/history.go Outdated
Signed-off-by: Albert Callarisa <albert@diagrid.io>
@acroca acroca marked this pull request as ready for review April 10, 2026 06:11
@acroca acroca requested review from a team as code owners April 10, 2026 06:11
@acroca acroca marked this pull request as draft April 10, 2026 07:56
@acroca
Copy link
Copy Markdown
Member Author

acroca commented Apr 10, 2026

Waiting for #1629 to be merged

Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
acroca added 2 commits May 11, 2026 17:16
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
@acroca acroca marked this pull request as ready for review May 12, 2026 09:17
@nelson-parente nelson-parente requested a review from Copilot May 19, 2026 20:30
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

// WTimer calls ctx.CreateTimer which produces origin=createTimer.
_, err := cmdWorkflowRun(appID, "WTimer", "--instance-id=timer-origin-test")
require.NoError(t, err)

Copy link
Copy Markdown
Contributor

@cicoyle cicoyle left a comment

Choose a reason for hiding this comment

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

changes mostly lgtm but there are test failures still from this PR - please take a look and address it and copilots feedback:

        time="2026-05-12T16:21:38.006246853Z" level=info msg="Workflow Actor 'child-retry-origin-test:0002': workflow completed with status 'ORCHESTRATION_STATUS_FAILED' workflowName 'FailingChildWorkflow'" app_id=test-workflow instance=runnervmeorf1 scope=dapr.runtime.actors.targets.orchestrator type=log ver=1.18.0-rc.3

The 4 TestWorkflowHistory/timer_origin_* subtests have been failing in
CI because the test worker app in tests/apps/workflow was still pinned
to github.com/dapr/durabletask-go v0.10.0. That release predates the
addition of the CreateTimerAction.Origin field, so even though the
runtime and the CLI both decode Origin correctly, the worker never
sets it -- the resulting TimerCreatedEvent has no Origin and the
history command shows no `origin=` attr, so require.Eventually times
out with "Condition never satisfied".

Bump the worker app's durabletask-go to the same commit the CLI's main
go.mod uses (c4b7279, pre-rename so API surface is still compatible
with go-sdk v1.13.0). That version emits Origin for createTimer,
externalEvent, activityRetry, and childWorkflowRetry.

Also address Copilot's outstanding review comment by adding t.Cleanup
to each timer-origin subtest. WTimer creates a 10h timer and
EventWorkflow waits on an external event for 1h, so without cleanup
those instances stay RUNNING in the state store and leak into later
E2E runs. Each subtest now terminates and purges its own instance,
and the instance ID is hoisted into a local const for readability.

Signed-off-by: Nelson Parente <nelson_parente@live.com.pt>
@nelson-parente
Copy link
Copy Markdown
Contributor

Hi @acroca — I pushed d9317ad directly to this branch (maintainerCanModify=true) to address both halves of @cicoyle's review. Happy to revert if you'd rather drive this yourself.

Root cause of the 4 failing subtests

The TestWorkflowHistory/timer_origin_* subtests were timing out with Condition never satisfied. After tracing the wire path end-to-end, the failure is on the worker side, not the CLI:

  • tests/apps/workflow/go.mod still pinned github.com/dapr/durabletask-go v0.10.0 (released 2025-09-10).
  • v0.10.0 predates the CreateTimerAction.Origin field — the worker's createTimerInternal builds a CreateTimerAction with no Origin set.
  • The runtime's applier (backend/runtimestate/applier.go) does switch o := timerAction.GetOrigin().(type) { … default: // stays nil }, so it emits a TimerCreatedEvent with no Origin.
  • The CLI then reads back a TimerCreatedEvent whose Origin oneof is nil, timerOriginInfo returns nil, no origin= attr is added, and the polling assertion never finds the substring.

So the CLI code in this PR is correct; the worker just wasn't producing the field.

Fix

  1. tests/apps/workflow/go.mod + go.sum — bump durabletask-go from v0.10.0v0.11.4-0.20260413145313-c4b7279b6a8e (same commit cli/go.mod already uses, pre-rename so the API surface is still compatible with go-sdk v1.13.0). At this commit, the worker emits Origin for createTimer, externalEvent, activityRetry, and childWorkflowRetry.

  2. tests/e2e/standalone/workflow_test.go — addresses Copilot's outstanding comment about long-running instances leaking. WTimer creates a 10h timer and EventWorkflow waits on an external event for 1h, so without cleanup they stay RUNNING in the state store and pollute later E2E runs. Each subtest now has a t.Cleanup that terminates + purges its own instance, and the instance ID is hoisted into a local const for readability.

Verification done locally

  • go build ./... for both cli and tests/apps/workflow — pass
  • go vet -tags=e2e ./tests/e2e/standalone/... — clean
  • go test ./pkg/workflow/... — 20 tests pass

E2E will run on CI for the real verification.

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