temporalio / features

Behavior and history compatibility testing for Temporal SDKs
13 stars 16 forks source link

[Feature Request] Standardize method for listing workflow queries/signals (and maybe other things like registered activities/workflows and other metadata) #51

Open bergundy opened 2 years ago

bergundy commented 2 years ago

Up until now we've used a hack to get the list of registered queries from a workflow execution. The hack is to send a query to a workflow, which responds with something like "query not found, list of registered queries...", parse the response and show in the UI.

A standard way to support listing workflow queries would be to have a "listQueries" (name TBD) built-in query provided by the Workflow runtime.

We'd have to keep the hack around to support SDKs that still do not support the listQueries built-in query (all at the time of this writing).

UPDATE: Here is the roadmap:

cretz commented 2 years ago

There are other things that are only registered at the SDK level, e.g. signal handlers. I wonder if we want a more generic "get SDK workflow details" query that also shows signal handlers and potentially any other SDK-side-only metadata. Whatever is decided, of course it needs to be well specified since all SDKs need to support it in the exact same way.

bergundy commented 2 years ago

That's a good suggestion @cretz, let's do that.

Sushisource commented 2 years ago

We should define the message in the API proto repo

feedmeapples commented 2 years ago

is it possible in theory to also provide input data type definitions? for both Queries and Signals in case we want to dynamically construct the UI

cretz commented 2 years ago

@feedmeapples - Can discuss in separate issue/discussion as that's much more complex (and due to signals persisting in history even on bad input poses some issues alleviated by the upcoming synchronous update feature).

cretz commented 1 year ago

Implementation Thoughts

This should be represented as proto which every language and the UI can use. Granted the format will be JSON-based proto not binary.

Here would be my idea (expressed in TS terms, but applicable to all SDKs):

// This can be per workflow run or per workflow type
message WorkflowMetadata {
  WorkflowDefinition definition = 1;

  // TODO(cretz): Future possibilities
  // * Stack information
  // * Worker metadata sans workflow list
}

message WorkflowDefinition {
  // If empty, this is a "dynamic" form of the workflow
  string type = 1;
  // Optional
  string description = 2;

  // When fetching at a worker level these are obviously fixed to the workflow
  // definition which means that languages like Go need to provide a way to
  // allow developers to define these things ahead of time if they want (unlike
  // today which is runtime). When fetching at a workflow level, these may be
  // specific to a run since handlers can be added inside of workflows.
  //
  // We can break these out to separate message types for each definition type
  // if/when there are fields specific to each.
  repeated InteractionDefinition signal_definitions = 3;
  repeated InteractionDefinition query_definitions = 4;
  repeated InteractionDefinition update_definitions = 5;
}

message InteractionDefinition {
  // If empty, this is a "dynamic" form of the interaction
  string name = 1;

  // Optional. While we can support type hinting information for callers in
  // more well-typed fields in the future, it gets complicated quickly.
  // Therefore, all information a caller may need should currently be in English
  // in this field (even if it's generated by the SDK).
  string description = 2;
}

message ActivityDefinition {
  // If empty, this is a "dynamic" form of the activity
  string type = 1;
  // Optional
  string description = 2;
}

message WorkerMetadata {
  repeated WorkflowDefinition workflow_definitions = 1;
  repeated ActivityDefinition activity_definitions = 2;

  // TODO(cretz): There are so many options here to add things such as:
  // * Registration/config options
  // * Cache info
  // * Identity information we usually have for workers
  // * SDK language and version
  // * General process/metric info (but don't overdo this)
}

Here are the calls one can make:

For discussion:

  1. Is "metadata" a good word? Even though we are only talking about "definitions" here, I want to encompass the idea of runtime information that is not necessarily definition based
  2. Maybe we should break "description" into "summary" and "description" to encourage one-liner "summary" for use in UI listing and other places
  3. Is there value yet in asking an activity for runtime "metadata" per activity run (obv as a request on a heartbeat response)?
  4. How does each language represent its "description" information. Some languages like Python can do neat things like grab the __doc__ and also add type hint information, but most other languages do not have docs at runtime so a separate field at the registration/declaration/definition/decoration/annotation/attribute site would be required
  5. How best to allow Go to predefine its interactions?
  6. Be sure to ask UI team if this works for them and/or if they have any suggestions
  7. For an initial version of this, we can just support the "get workflow metadata" operation as a query and ignore all that "worker update" business, but we need to keep it in mind so we design our metadata approach where it could be obtained at a worker level
Alex-Tideman commented 11 months ago

I would love to get this listQueries feature in all the SDKs. We are envisioning building custom UIs based on query names and it would be a lot nicer to do this without the current hack. Happy to work with whoever on this from the SDK side.

antlai-temporal commented 9 months ago
  // If empty, this is a "dynamic" form of the workflow

@cretz I wonder if we need to report dynamic workflow/interactions at all. In particular, if they don't have a description either, there is no proper way to identify them...

cretz commented 9 months ago

I wonder if we need to report dynamic workflow/interactions at all. In particular, if they don't have a description either, there is no proper way to identify them...

This is about providing information to the caller about the shape of what is defined on the workflow. The caller may want to know that any query could be accepted because dynamic query is enabled. This can affect things like the UI where they may make it clear to a user that any arbitrary string query name would be accepted if they knew that dynamic queries were supported.

Btw, if you are tackling this now, I would ignore anything about "worker queries" in this issue, that can be done somewhere else.

antlai-temporal commented 9 months ago

. The caller may want to know that any query could be accepted because dynamic query is enabled Do we have the same concept for updates and signals, or just only for queries? The examples I'm seeing define a generic signal first, and add extra info in the payload, so name will not be missing... Mistery solved, dynamic handlers are not supported in TS, only Java/Python/.Net, and go uses GetUnhandledSignalNames for signals. For update, there is no GetUnhandledUpdateNames in go (I'm adding an issue), Java seems to support it, not sure about .Net/Python...

cretz commented 9 months ago

This issue is known and #201 exists to bring parity here.

Signals in Go for this feature request are going to be a bit interesting. My suggestion: there is no such thing as a signal definition in Go so we should never populate that repeated field in metadata. However, we can create helper API in Go like workflow.DefineSignalChannel(workflow.SignalDefinition{Name: "my-signal", Description: "whatever"}) and people can call that to populate this info but it won't relate to their actual channel use. It would be a bad idea IMO to populate Go signal definitions in this metadata just based on whether a signal channel was ever asked for and similarly bad if it included any signal name that had been seen.

We're going to need to design how to provide "descriptions" to these interactions in every language anyways. In Go, SetUpdateHandlerWithOptions's options can have it, but you'd need a new SetQueryHandlerWithOptions for queries. In Java, Python, and .NET you can use a field on the annotation, decorator, and attribute respectively though you need to still accept them on runtime-added forms. In TS these are likely just options on the define calls.

I can provide clarification here if unclear.

Quinn-With-Two-Ns commented 7 months ago

This needs to be done in every SDK other than TypeScript

Sushisource commented 3 months ago

Note from UI sync: Good idea to have an "is visible in UI" flag for interactions

Other possible future options: