actions metering metadata proto definition #701
Conversation
|
|
| // A history event can be attributed to multiple metering subtypes | ||
| // Ex. EVENT_TYPE_WORKFLOW_EXECUTION_STARTED, which is used by both Start | ||
| // Workflow Execution and Scheduled Workflow Executions. | ||
| // grpc:StartWorkflowExecution and grpc:StartWorkflowExecution.Scheduled.Adjusted | ||
| repeated Action actions = 1; |
There was a problem hiding this comment.
This is a bit confusing to read as a public consumer of the API. Not sure "metering subtypes" and such make sense here. From an outsider POV, can you help me understand when this field would be set and on which history events?
There was a problem hiding this comment.
Yeah this is a starter but definitely will need a more comprehensive explanation. The idea is that history events are attributed to a potentially billable aspect. For example, if we come across a EVENT_TYPE_WORKFLOW_EXECUTION_STARTED, we might see an action count of 1, or even 3 (if we find that it was actually started via a scheduled workflow instead of a regular start wf exectution). We can, alternatively, have history events with an action of 0, which would mean they are not billable. We're planning on intercepting frontend requests to enrich history event data with a corresponding action count.
There was a problem hiding this comment.
I do not think you should store redundant action information in history if it can be reliably derived from history based on known events. Adding this redundant information affects everyone's history format that they store on disk for several uses and it's unnecessary information.
We should instead just store action information that otherwise cannot be derived from history (e.g. queries and heartbeats). And IMO that information doesn't need to be on history anyways, it can be part of general workflow information.
There was a problem hiding this comment.
I agree that this information can be derived from history and it may be redundant, but the objective here is to improve billing visibility so that by having this metadata readily available in history, users would not have to implement their own billing analyzer logic in order for them to calculate their Temporal usages/bills. To minimize polluting this field, we are ideally starting small by only including the most relevant fields required.
There was a problem hiding this comment.
Attaching some resources that may provide a bit more context on the initiative:
- https://www.notion.so/temporalio/HistoryEvent-Metadata-Field-Proposal-2ee8fc567738809b88bae68690d39eac?source=copy_link
- https://temporaltechnologies.slack.com/archives/C01RN061UMR/p1764704039212379
- https://www.notion.so/temporalio/Improving-Billing-Visibility-for-Actions-2bd8fc56773880c1a41ee3845c9966de?source=copy_link
There was a problem hiding this comment.
This is a public repo, internal links may not make as much sense, can discuss internally
users would not have to implement their own billing analyzer logic in order for them to calculate their Temporal usages/bills
No, we can implement this for them. Someone already has to implement some logic to count the actions, how is counting the events different?
There was a problem hiding this comment.
Here are a few other potential options if we feel strongly about not adding metering metadata to the history event:
-
GetWorkflowExecutionHistory takes some form of verbosity flag or argument, which, if specified, only populates the data on the history event then.
-
We add a separate endpoint that takes in a (workflowid, runid) and returns billable information/metadata about the Workflow.
Any preferences between the above two, or are there any other options we are missing
| option java_multiple_files = true; | ||
|
|
||
| message ActionMetadata { | ||
| // A history event can be attributed to multiple metering subtypes |
| // Ex. EVENT_TYPE_WORKFLOW_EXECUTION_STARTED, which is used by both Start | ||
| // Workflow Execution and Scheduled Workflow Executions. | ||
| // grpc:StartWorkflowExecution and grpc:StartWorkflowExecution.Scheduled.Adjusted | ||
| repeated Action actions = 1; |
There was a problem hiding this comment.
The idea behind using a repeated message here is that we can have multiple actions attributed to a HistoryEvent. This makes it easier to extend later down the road (ie if we want to add more metadata to the Action message such as action_id or display_category).
| // A history event can be attributed to multiple metering subtypes | ||
| // Ex. EVENT_TYPE_WORKFLOW_EXECUTION_STARTED, which is used by both Start | ||
| // Workflow Execution and Scheduled Workflow Executions. | ||
| // grpc:StartWorkflowExecution and grpc:StartWorkflowExecution.Scheduled.Adjusted |
|
Closing as opened up a new PR here #703 |
READ BEFORE MERGING: All PRs require approval by both Server AND SDK teams before merging! This is why the number of required approvals is "2" and not "1"--two reviewers from the same team is NOT sufficient. If your PR is not approved by someone in BOTH teams, it may be summarily reverted.
What changed?
Why?
Breaking changes
Server PR