Experimental user metadata and workflow metadata query support #1597

cretz · 2024-08-16T22:20:20Z

What was changed

Added StartWorkflowOptions.StaticSummary and StartWorkflowOptions.StaticDetails fields that are populated on workflow start event and execution config
Added workflow.SetCurrentDetails and workflow.GetCurrentDetails functions that set current details available on workflow metadata query
Added workflow.NewTimerWithOptions that accepts a TimerOptions which has a Summary to be put on the timer event
- Note, this required a new outbound interceptor method and may have broken compile for PHP SDK internalbindings (by intention)
Added workflow.SetQueryHandlerWithOptions that accepts a QueryHandlerOptions which has a Description
Added workflow.GetSignalChannelWithOptions that accepts a SignalChannelOptions which has a Description
Added support for __temporal_workflow_metadata query that returns query, signal, and update definitions and the current details

Note, some things are not supported:

Child workflow user metadata - User metadata not propagated from child workflow command to child workflow's execution config temporal#6412
Scheduled workflow user metadata - User metadata not propagated from schedule action to workflow's execution config temporal#6413
Continue as new user metadata - known and to be discussed
Set summaries for internal timers - Update internal timers to use NewTimerWithOptions and a summary #1602

Checklist

Closes User metadata and workflow metadata query #1565

Fixes temporalio#1565

internal/internal_schedule_client.go

cretz · 2024-08-16T22:21:22Z

internal/interceptor.go

@@ -231,6 +231,9 @@ type WorkflowOutboundInterceptor interface {
 	// NewTimer intercepts workflow.NewTimer.
 	NewTimer(ctx Context, d time.Duration) Future

+	// NewTimer intercepts workflow.NewTimerWithOptions.
+	NewTimerWithOptions(ctx Context, d time.Duration, options TimerOptions) Future


Based on available options, a new outbound method seemed like the best approach here. Should be safe since we require the base to be embedded.

cretz · 2024-08-16T22:22:58Z

internal/internal_command_state_machine.go

+	if options.Summary != "" {
+		var err error
+		if summary, err = dc.ToPayload(options.Summary); err != nil {
+			panic(err)


This should be rare (it's just serializing string), but there is precedent with other commandsHelper calls panicking if they can't convert.

internal/internal_workflow.go

cretz · 2024-08-16T22:25:28Z

internal/internal_time.go

@@ -45,7 +45,7 @@ type (

 		// NewTimer - Creates a new timer that will fire callback after d(resolution is in seconds).
 		// The callback indicates the error(TimerCanceledError) if the timer is canceled.
-		NewTimer(d time.Duration, callback ResultHandler) *TimerID
+		NewTimer(d time.Duration, options TimerOptions, callback ResultHandler) *TimerID


This may break PHP SDK since it's exposed via internalbindings but unsure if they use it. This seemed like the best way and should cause a clear compile break.

I think they do but it should be fine

Quinn-With-Two-Ns · 2024-08-16T22:39:50Z

internal/interceptor.go

@@ -231,6 +231,9 @@ type WorkflowOutboundInterceptor interface {
 	// NewTimer intercepts workflow.NewTimer.
 	NewTimer(ctx Context, d time.Duration) Future

+	// NewTimer intercepts workflow.NewTimerWithOptions.


Can other primitives support metadata like activities? Do we plan to add support for them?

Yes and yes! We intentionally decided that workflow summary/details and timer summary was the first use case. But the user metadata is on the command/event and not the attributes for this reason. Any UI/CLI-visible metadata you could ever need on an event we can look into adding.

internal/interceptor.go

internal/client.go

Sushisource

I thought we had the ability to add this to child workflows straight away too?

internal/internal_command_state_machine.go

Sushisource · 2024-08-16T23:13:33Z

internal/internal_time.go

@@ -45,7 +45,7 @@ type (

 		// NewTimer - Creates a new timer that will fire callback after d(resolution is in seconds).
 		// The callback indicates the error(TimerCanceledError) if the timer is canceled.
-		NewTimer(d time.Duration, callback ResultHandler) *TimerID
+		NewTimer(d time.Duration, options TimerOptions, callback ResultHandler) *TimerID


I think they do but it should be fine

internal/internal_workflow.go

Sushisource · 2024-08-16T23:20:21Z

internal/schedule_client.go

@@ -281,6 +281,23 @@ type (
 		//
 		// Deprecated - This is only for update of older search attributes. This may be removed in a future version.
 		UntypedSearchAttributes map[string]*commonpb.Payload
+
+		// Summary - Single-line summary for this workflow execution that will appear in UI/CLI. This can be in
+		// single-line Temporal markdown format.


None of these strings that reference this markdown format have info about what it is. Probably because it doesn't exist yet, but, if it does somewhere, link would be good.

Yeah, as this concept is stabilized (i.e. non-experimental) and we have something at docs.temporal.io, would definitely want to link it here.

cretz · 2024-08-19T14:28:17Z

I thought we had the ability to add this to child workflows straight away too?

Yes, will add

cretz · 2024-08-19T16:27:23Z

Actually I am not seeing child workflow as working. I put in a bunch of work to wire it up for child workflows then my integration tests show it didn't propagate. I will get with authors of temporalio/temporal#5857, but in the meantime I will add everything but the final user-facing option.

# Conflicts: # .github/workflows/ci.yml # test/workflow_test.go

cretz · 2024-08-22T19:27:13Z

Updated to change to StaticSummary/StaticDetails for workflows

gow · 2024-08-29T19:57:07Z

internal/internal_schedule_client.go

+		if err != nil {
+			return nil, err
+		}
+


We need a similar change when the schedule is updated. The UpdateSchedule API on the server is structured to completely replace the schedule.Action with the value provided in the update API. So if the client wants to update a single field, it should echo back the whole schedule with the updated field. The client already seems to be doing that.
But in my testing I noticed that the usermetadata is not being copied over between these two calls - https://github.com/temporalio/sdk-go/blob/master/internal/internal_schedule_client.go#L276-L280. Looks like ScheduleWorkflowAction struct needs some updates?
cc @dnr

This same code is used for update. The way update works in SDKs is it's a get-then-update which calls these same helpers to go from user model to proto model.

Experimental user metadata support

0c43849

Fixes temporalio#1565

cretz requested a review from a team as a code owner August 16, 2024 22:20

cretz commented Aug 16, 2024

View reviewed changes

Quinn-With-Two-Ns reviewed Aug 16, 2024

View reviewed changes

internal/interceptor.go Show resolved Hide resolved

Quinn-With-Two-Ns reviewed Aug 16, 2024

View reviewed changes

internal/client.go Outdated Show resolved Hide resolved

Sushisource reviewed Aug 16, 2024

View reviewed changes

cretz added 2 commits August 19, 2024 11:50

More user metadata features

16a6495

Disable schedule user metadata

a3f3971

cretz mentioned this pull request Aug 19, 2024

Update internal timers to use NewTimerWithOptions and a summary #1602

Closed

cretz added 2 commits August 19, 2024 12:17

Minor code comment improvement

75cba87

Merge remote-tracking branch 'remotes/origin/master' into user-metadata

e4cc987

# Conflicts: # .github/workflows/ci.yml # test/workflow_test.go

Sushisource approved these changes Aug 20, 2024

View reviewed changes

Change to StaticSummary/StaticDetails

17af51c

gow reviewed Aug 29, 2024

View reviewed changes

Quinn-With-Two-Ns approved these changes Aug 29, 2024

View reviewed changes

Merge branch 'master' into user-metadata

d32b800

cretz merged commit f47e644 into temporalio:master Aug 30, 2024
13 checks passed

cretz deleted the user-metadata branch August 30, 2024 14:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Experimental user metadata and workflow metadata query support #1597

Experimental user metadata and workflow metadata query support #1597

cretz commented Aug 16, 2024 •

edited

Loading

cretz Aug 16, 2024

cretz Aug 16, 2024

cretz Aug 16, 2024

Sushisource Aug 16, 2024

Quinn-With-Two-Ns Aug 16, 2024

cretz Aug 16, 2024

Sushisource left a comment

Sushisource Aug 16, 2024

Sushisource Aug 16, 2024

cretz Aug 19, 2024

cretz commented Aug 19, 2024

cretz commented Aug 19, 2024 •

edited

Loading

cretz commented Aug 22, 2024

gow Aug 29, 2024

cretz Aug 29, 2024

Experimental user metadata and workflow metadata query support #1597

Experimental user metadata and workflow metadata query support #1597

Conversation

cretz commented Aug 16, 2024 • edited Loading

What was changed

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sushisource left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cretz commented Aug 19, 2024

cretz commented Aug 19, 2024 • edited Loading

cretz commented Aug 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cretz commented Aug 16, 2024 •

edited

Loading

cretz commented Aug 19, 2024 •

edited

Loading