Discuss: namespace rules prototype #532

ychebotarev · 2025-01-27T18:05:09Z

What changed?
This is a proposal for "namespace level rules". First implementation will be "activity pause".
Each rule is defined by:

predicate - SQL like (or "visibility-like") filtering conditions. Format is specific for provided type.
type. Type defines both triggering condition and available columns. Right now the only available one is "activity".
action. Define the action. Currently mostly activity related. Will be expanded to "workflow pause", and then for other rules.

Why?
Part of the operational efforts. In case of ongoing issues users don't only want to affect (pause for example) running workflows/activities, but also prevent incoming workflows/activities from running into the same issues.
But based on the feedback the scope can be wider, and this can be solved in more generable way. Thus this proposal.

temporal/api/enums/v1/namespace.proto

cretz · 2025-01-28T14:54:45Z

temporal/api/namespace/v1/message.proto

+// NamespaceRule describes a rule that can be to any workflow in this namespace.
+message NamespaceRule {


That sounds more like NamespaceWorkflowRule. Or is it intended to possibly house rules about more than just workflows (and their activities)?

for now I think it will be restricted to workflow.

more like:

I can't imagine this to be on a higher level then workflows

even if it is I probably want them to be separated

I will rename it.

Or not. This can be used to pause TaskQueue for example. Technically it is not a part of workflow.

Overall - yes, NamespaceRule fills like scope is too big. But NamespaceWorkflowRule feels like scope is too narrow.

I like NamespaceRule the most.

We should update the doc that says it's only intended for workflows (unsure if we can expect such a thing when the expected scope-expanse occurs)

temporal/api/namespace/v1/message.proto

temporal/api/workflowservice/v1/service.proto

temporal/api/namespace/v1/message.proto

alexshtin · 2025-01-28T22:50:04Z

temporal/api/enums/v1/namespace.proto

+// I will remove it once we agree on the overall design
+// RULE_ACTION_TYPE_EMIT_VISIBILITY = 3;
+// RULE_ACTION_TYPE_PAUSE_WORKFLOW = 4;
+// RULE_ACTION_TYPE_SIGNAL = 5;


I would move them inside enum (still commented out) with comment saying something like here are other possible actions which might be implemented in future.

alexshtin · 2025-01-28T22:50:55Z

temporal/api/namespace/v1/message.proto

+    // I will remove it once we agree on the overall design.
+    // Payload of the rule action. The payload is specific to the rule action type.
+    // temporal.api.common.v1.Payload payload = 2;


Same. Leave it commented out. I think it is acceptable.

alexshtin · 2025-01-28T22:52:11Z

temporal/api/namespace/v1/message.proto

+// NamespaceRule describes a rule that can be to any workflow in this namespace.
+message NamespaceRule {


I like NamespaceRule the most.

alexshtin · 2025-01-28T23:00:23Z

temporal/api/namespace/v1/message.proto

+    // Name of the rule.
+    string name = 1;
+
+    // ID must be unique within the namespace
+    string id = 2;


I think you should follow the same approach as we do for namespaces themselves. Id is hidden from the user (mostly). When user creates rule, it specifies only name and gets Id back in response. Then user can address rule by name (and by Id too). Yes, you can't guarantee uniqueness of the name. We need to discuss what we can do in this case.

temporal/api/workflowservice/v1/request_response.proto

alexshtin · 2025-01-28T23:02:32Z

temporal/api/workflowservice/v1/service.proto

+    }
+
+    // ApplyNamespaceRule allows to apply rule to a certain workflow instance without creating a rule.
+    // This is useful for one-off operations.


and for tests too. I mean users can test how feature works with test workflows.

alexshtin · 2025-01-29T01:23:21Z

temporal/api/workflowservice/v1/request_response.proto

+message DescribeNamespaceRuleRequest {
+    string namespace = 1;
+
+    string rule_id = 2;


It would be great if you use oneof for both id and name. And for Delete too.

cretz · 2025-01-29T15:56:18Z

temporal/api/namespace/v1/message.proto

+
+message RuleAction {
+    // Type of the rule action.
+    temporal.api.enums.v1.RuleActionType type = 1;


I think you should consider structured data for the action instead of an enum. Therefore if an action needs data, it can be properly modeled data and not arbitrary payload or whatever. I recommend a oneof with specific messages representing the actions.

yeah, that is exactly what I did, PR is not updated - I want to update it after the discussion.

cretz · 2025-01-29T15:57:04Z

temporal/api/namespace/v1/message.proto

+    // Type of the rule trigger. Right now only activity is supported.
+    // Trigger type defines when the rule should be evaluated.
+    // It also defines the columns available in the predicate.
+    // - activity: Rule will be triggered when an activity is about to start.


This doesn't sound like an ACTIVITY trigger, but rather an ACTIVITY_START trigger. There are many event situations in an activity.

cretz · 2025-01-29T15:57:40Z

temporal/api/enums/v1/namespace.proto

+    RULE_TRIGGER_TYPE_ACTIVITY = 1;
+    RULE_TRIGGER_TYPE_WORKFLOW = 2;


Can we define what these represent? And if workflow is not in use it does not need to be present.

cretz · 2025-01-29T15:58:11Z

temporal/api/enums/v1/namespace.proto

+enum RuleActionType {
+    RULE_ACTION_TYPE_UNSPECIFIED = 0;
+    RULE_ACTION_TYPE_PAUSE_ACTIVITY = 1;
+    RULE_ACTION_TYPE_EMIT_METRIC = 2;


Can you clarify what emit metric means exactly? There are lots of details to emitting a metric (metric name, labels, etc) but I can't find where such things would be set. Also, is that part of this project?

I was thinking about emitting metrics with predefined name, with dimensions like namespace, "type". May be with "rule name" as well.

Can you document the exact behavior of this enum? It was unclear to me and I suspect other users of the API what this means exactly? I do think this and other actions make sense as structured messages instead of just enumerates.

yeah, in the doc I moved from enum to "oneof" and messages, so we can have structured action payload later if we need to.

cretz · 2025-01-29T16:00:42Z

temporal/api/namespace/v1/message.proto

+    // Trigger type defines when the rule should be evaluated.
+    // It also defines the columns available in the predicate.
+    // - activity: Rule will be triggered when an activity is about to start.
+    temporal.api.enums.v1.RuleTriggerType trigger_type = 3;


I think the concept of "action" and the "trigger" should be merged. Some actions make no sense with some triggers I assume. I think an action should also define when it should take place alongside its other options.

there is no 1:1 mapping between "action" and "trigger". For example workflow pause can be triggered by activity rule.

cretz · 2025-01-29T16:01:40Z

temporal/api/enums/v1/namespace.proto

+// RULE_ACTION_TYPE_EMIT_VISIBILITY = 3;
+// RULE_ACTION_TYPE_PAUSE_WORKFLOW = 4;
+// RULE_ACTION_TYPE_SIGNAL = 5;
+enum RuleActionType {


Suggested change

enum RuleActionType {

enum NamespaceRuleActionType {

Not sure it's safe to assume every use of the term "rule" will apply to namespace rules (same in other places where not qualified)

cretz · 2025-01-29T16:02:22Z

temporal/api/workflowservice/v1/request_response.proto

+    // If true, the rule will be applied immediately via batch job.
+    bool force_apply = 3;
+
+    // the workflow query to use to apply the rule


Pedantic, but there is a lot of inconsistent with capitalization of comments, punctuation, etc

cretz · 2025-01-29T16:03:28Z

temporal/api/workflowservice/v1/request_response.proto

+    bool force_apply = 3;
+
+    // the workflow query to use to apply the rule
+    string workflow_query = 4;


Isn't this already in the rule?

I go back and force on this a lot...
we will discuss it offline

cretz · 2025-01-29T16:05:58Z

temporal/api/namespace/v1/message.proto

+    // - backoffInterval: The current backoff interval of the activity.
+    // - activityStatus: The status of the activity. Can be one of "scheduled", "started", "paused".
+    // - taskQueue: The name of the task queue of the activity.
+    string predicate = 4;


What is a "predicate"? Is this some kind of new query format? Is it possible to use well-typed structured representations of this? If not, I would at least request a grammar (we didn't have it for the longest time for workflow query and people were confused for a long time), but ideally we are stronger typed and more structural than that.

Discuss: namespace rules prototype

7d28789

ychebotarev requested review from a team as code owners January 27, 2025 18:05

ychebotarev assigned alexshtin Jan 27, 2025

cretz reviewed Jan 28, 2025

View reviewed changes

ychebotarev added 2 commits January 28, 2025 12:39

work on comments/feedback

095bb20

Add ApplyNamespaceRules intent and comments

9349b13

ychebotarev requested a review from cretz January 28, 2025 20:45

alexshtin reviewed Jan 28, 2025

View reviewed changes

fix execution_info field

1a17ad6

alexshtin reviewed Jan 29, 2025

View reviewed changes

cretz reviewed Jan 29, 2025

View reviewed changes

ychebotarev closed this Feb 5, 2025

ychebotarev mentioned this pull request Feb 5, 2025

Discuss: namespace rules prototype #550

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Discuss: namespace rules prototype #532

Discuss: namespace rules prototype #532

ychebotarev commented Jan 27, 2025 •

edited

Loading

cretz Jan 28, 2025 •

edited

Loading

ychebotarev Jan 28, 2025

ychebotarev Jan 28, 2025

ychebotarev Jan 28, 2025

ychebotarev Jan 28, 2025

ychebotarev Jan 28, 2025

alexshtin Jan 28, 2025

cretz Jan 29, 2025

alexshtin Jan 28, 2025

alexshtin Jan 28, 2025

alexshtin Jan 28, 2025

alexshtin Jan 28, 2025

alexshtin Jan 28, 2025

alexshtin Jan 29, 2025

cretz Jan 29, 2025

ychebotarev Feb 3, 2025

cretz Jan 29, 2025

cretz Jan 29, 2025

cretz Jan 29, 2025

ychebotarev Jan 30, 2025

cretz Feb 3, 2025

ychebotarev Feb 3, 2025

cretz Jan 29, 2025

ychebotarev Jan 30, 2025

cretz Jan 29, 2025

cretz Jan 29, 2025

cretz Jan 29, 2025

ychebotarev Feb 3, 2025

cretz Jan 29, 2025

		// NamespaceRule describes a rule that can be to any workflow in this namespace.
		message NamespaceRule {

		RULE_TRIGGER_TYPE_ACTIVITY = 1;
		RULE_TRIGGER_TYPE_WORKFLOW = 2;

Discuss: namespace rules prototype #532

Discuss: namespace rules prototype #532

Conversation

ychebotarev commented Jan 27, 2025 • edited Loading

cretz Jan 28, 2025 • edited Loading

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

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

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

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

Choose a reason for hiding this comment

ychebotarev commented Jan 27, 2025 •

edited

Loading

cretz Jan 28, 2025 •

edited

Loading