User metadata on workflows/events for user interface usage #371

cretz · 2024-03-18T19:46:37Z

What changed?

Added temporal.api.sdk.v1.UserMetadata containing payloads for summary and details for general metadata use
Added temporal.api.command.v1.Command.user_metadata for general use (initially only for timers and child workflow)
Added temporal.api.history.v1.HistoryEvent.user_metadata for general use (initially only for timers)
Added temporal.api.workflowservice.v1.StartWorkflowExecutionRequest.user_metadata for general use (initially for populating execution config)
Added temporal.api.workflowservice.v1.SignalWithStartWorkflowExecutionRequest.user_metadata for same reason as start
Added temporal.api.workflow.v1.NewWorkflowExecutionInfo.user_metadata for same reason as start
Added temporal.api.workflow.v1.WorkflowExecutionConfig.user_metadata for general use (initially for fixed as-of-start summary/details)
Added temporal.api.sdk.v1.WorkflowMetadata.current_details for the ability to have queryable long-form text representing current details of a workflow
Removed temporal.api.sdk.v1.WorkflowDefinition.definition because a workflow definition's description isn't best placed on a query (but "current details" is good on the metadata itself)

Why?

Users need a way to set fixed summary/details when starting a workflow for use by UIs
Users need a way to have query-obtainable live "current details" for use by UIs
Users need a way to set a summary when starting a timer for use by UIs
All of this needs to be encrypted if the user wants
We need to take into account future user-metadata use

Breaking changes

Yes for workflow metadata which was only used in JSON form by one SDK (TypeScript) and not even invoked by UI yet

cretz · 2024-03-18T19:49:42Z

temporal/api/sdk/v1/user_metadata.proto

+  // Short-form text that provides a summary. This payload should be a "json/plain"-encoded payload
+  // that is a single JSON string. Formatting may not apply to this text when used in "title"
+  // situations. The payload data section is currently limited to 400 bytes.
+  temporal.api.common.v1.Payload summary = 1;
+
+  // Long-form text that provides details. This payload should be a "json/plain"-encoded payload
+  // that is a single JSON string. Formatting may apply to this text in common use. The payload data
+  // section is currently limited to 20000 bytes.
+  temporal.api.common.v1.Payload details = 2;


Notes:

Considered "description" instead of "details", but description isn't a general-purpose-enough term

The 400 and 20000 bytes is an arbitrary ceiling based on expected use, happy to change

These are payloads so they can be encrypted

The use of JSON string as the type is a simple compatibility thing

At this time, we are not defining the long-form format (but I suspect some very limited form of markdown)

cretz · 2024-03-18T19:50:14Z

temporal/api/sdk/v1/workflow_metadata.proto

This file is safe for reworking in an incompatible way because it's just models for a well-known workflow query. See description to understand the changes.

bergundy

All of the fields are just plain text at the moment, right?
Would be nice to understand what the format is.

temporal/api/command/v1/message.proto

temporal/api/sdk/v1/user_metadata.proto

bergundy · 2024-03-18T19:53:34Z

temporal/api/sdk/v1/workflow_metadata.proto

@@ -35,20 +35,19 @@ option csharp_namespace = "Temporalio.Api.Sdk.V1";
 message WorkflowMetadata {
  // Metadata provided at declaration or creation time.
  WorkflowDefinition definition = 1;
+  // Current long-form details of the workflow's state. This is used by user interfaces to show
+  // long-form text. This text may be formatted by the user interface.
+  string current_details = 2;


This is always returned in query responses and goes through the user's DC, right?

bergundy · 2024-03-18T19:55:06Z

temporal/api/sdk/v1/workflow_metadata.proto

-  // An optional workflow description provided by the application.
-  // By convention, external tools may interpret its first part,
-  // i.e., ending with a line break, as a summary of the description.
-  string description = 2;


It this okay to break? Do we have any consumers of this as protos?

Yes it's ok to break, no we don't have any consumers of it except TypeScript SDK internally (and not this field and we only use this as JSON not proto). Explained in description and in comment.

temporal/api/sdk/v1/user_metadata.proto

cretz · 2024-03-18T20:02:16Z

All of the fields are just plain text at the moment, right?

Yes, this is documented on the field

Would be nice to understand what the format is.

Mentioned here, I am intentionally leaving that as out-of-scope for now pending some UI discussions. We should not block this progress on that.

Sushisource

Looking good to me, just minor stuff.

temporal/api/command/v1/message.proto

temporal/api/sdk/v1/user_metadata.proto

temporal/api/workflow/v1/message.proto

temporal/api/command/v1/message.proto

dnr · 2024-03-19T00:53:57Z

temporal/api/sdk/v1/workflow_metadata.proto

@@ -35,20 +35,19 @@ option csharp_namespace = "Temporalio.Api.Sdk.V1";
 message WorkflowMetadata {
  // Metadata provided at declaration or creation time.
  WorkflowDefinition definition = 1;
+  // Current long-form details of the workflow's state. This is used by user interfaces to show
+  // long-form text. This text may be formatted by the user interface.
+  string current_details = 2;


Why a Payload in history and a string here?

Because this object is a query result and query results are payloads already (so already subject to encryption in transit) whereas in history if you want something encrypted at rest it needs to be a payload.

dnr · 2024-03-19T00:59:09Z

temporal/api/sdk/v1/user_metadata.proto

+  // Short-form text that provides a summary. This payload should be a "json/plain"-encoded payload
+  // that is a single JSON string. Formatting may not apply to this text when used in "title"
+  // situations. The payload data section is limited to 400 bytes by default.
+  temporal.api.common.v1.Payload summary = 1;


I assume we considered this as a map from string to Payload.. why not do that? (For the record I like this better, just wondering what the reasoning was.)

There were a few of reasons that was rejected (there's an internal document discussing this). The primary reasons were that 1) the stringly-typed contract was too loose, 2) the keys are not user-defined, and 3) it unnecessarily increased history size for fixed keys.

temporal/api/workflow/v1/message.proto

buf.yaml

# Conflicts: # buf.yaml

gow · 2024-04-26T01:50:28Z

temporal/api/command/v1/message.proto

+    //    are used by user interfaces to show fixed as-of-start workflow summary and details.
+    //  * start_timer_command_attributes - populates temporal.api.history.v1.HistoryEvent for timer
+    //    started where the summary is used to identify the timer.
+    temporal.api.sdk.v1.UserMetadata user_metadata = 301;


why choose the field number 301? Why not 18 (next immediate available number) or 100 (if we want to group in hundreds)?
Just curious about the convention we use when picking field numbers in proto.

Same reason why the person who added HistoryEvent.worker_may_ignore as 300, so that it would not interfere with our continually added, incrementally numbered attribute fields below. I chose 301 instead of 300 just to match history event's equivalent, but don't have to. The field number doesn't really matter much here, so it leaves room for attributes to keep adding.

# Conflicts: # buf.yaml # openapi/openapiv3.yaml

User metadata on workflows/events for user interface usage

db555cc

cretz requested review from a team as code owners March 18, 2024 19:46

cretz commented Mar 18, 2024

View reviewed changes

Line ending fixes

2da3c29

bergundy reviewed Mar 18, 2024

View reviewed changes

PR suggested fixes

d5b9376

cretz mentioned this pull request Mar 18, 2024

User metadata temporalio/features#141

Closed

Sushisource reviewed Mar 18, 2024

View reviewed changes

temporal/api/command/v1/message.proto Outdated Show resolved Hide resolved

temporal/api/sdk/v1/user_metadata.proto Outdated Show resolved Hide resolved

temporal/api/workflow/v1/message.proto Outdated Show resolved Hide resolved

bergundy approved these changes Mar 18, 2024

View reviewed changes

yiminc reviewed Mar 19, 2024

View reviewed changes

temporal/api/command/v1/message.proto Show resolved Hide resolved

Sushisource approved these changes Mar 19, 2024

View reviewed changes

dnr reviewed Mar 19, 2024

View reviewed changes

PR-suggested comments

3839519

cretz commented Mar 19, 2024

View reviewed changes

temporal/api/workflow/v1/message.proto Outdated Show resolved Hide resolved

cretz changed the title ~~[DO NOT MERGE] User metadata on workflows/events for user interface usage~~ User metadata on workflows/events for user interface usage Mar 26, 2024

tdeebswihart reviewed Mar 26, 2024

View reviewed changes

buf.yaml Outdated Show resolved Hide resolved

Move metadata from execution info to execution config

cb07811

cretz force-pushed the user-metadata branch from 6974c39 to cb07811 Compare April 3, 2024 13:01

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

3d93adc

# Conflicts: # buf.yaml

cretz requested a review from a team April 16, 2024 16:51

gow approved these changes Apr 29, 2024

View reviewed changes

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

c89cf6e

# Conflicts: # buf.yaml # openapi/openapiv3.yaml

stephanos mentioned this pull request May 16, 2024

Handle User Metadata in workflow start requests temporalio/temporal#5937

Merged

yiminc approved these changes May 17, 2024

View reviewed changes

cretz added 2 commits May 17, 2024 14:10

Merge branch 'master' into user-metadata

ae77172

Regen OpenAPI specs

1e8c2d4

cretz merged commit de9e005 into temporalio:master May 17, 2024
3 checks passed

cretz deleted the user-metadata branch May 17, 2024 19:53

cretz mentioned this pull request May 22, 2024

User metadata and workflow metadata query temporalio/features#486

Open

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User metadata on workflows/events for user interface usage #371

User metadata on workflows/events for user interface usage #371

cretz commented Mar 18, 2024 •

edited

Loading

cretz Mar 18, 2024

cretz Mar 18, 2024

bergundy left a comment

bergundy Mar 18, 2024

cretz Mar 18, 2024

bergundy Mar 18, 2024

cretz Mar 18, 2024 •

edited

Loading

cretz commented Mar 18, 2024 •

edited

Loading

Sushisource left a comment

dnr Mar 19, 2024

cretz Mar 19, 2024

dnr Mar 19, 2024

cretz Mar 19, 2024

gow Apr 26, 2024

cretz Apr 29, 2024

User metadata on workflows/events for user interface usage #371

User metadata on workflows/events for user interface usage #371

Conversation

cretz commented Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bergundy 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 Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

cretz commented Mar 18, 2024 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cretz commented Mar 18, 2024 •

edited

Loading

cretz Mar 18, 2024 •

edited

Loading

cretz commented Mar 18, 2024 •

edited

Loading