-
Notifications
You must be signed in to change notification settings - Fork 75
Add worker visibility API - heartbeat and list worker satus #599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
temporal/api/worker/v1/message.proto
Outdated
int64 memory_usage_bytes = 19; | ||
|
||
// A Workflow Task found a cached Workflow Execution to run against. | ||
int32 sticky_cache_hit = 20; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these counters since process start or since the last heartbeat?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this is something we already have: https://docs.temporal.io/references/sdk-metrics
So I guess since process start.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do need clear delineation in docs between values that are reset every heartbeat and ones that are cumulative
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I will add 'total/last/current' qualifies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keeping this open
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're pretty much there. Only a few non-doc changes here.
IMO, let's add this to the polls too, since I know that's something we're going to want to do in the first SDK implementation anyway.
<!-- Describe what has changed in this PR --> **What changed?** What changed? * 2 new APIs added: * WorkerHeartbeat * ListWorkerStatus * WorkerInfo information added to: * ShutDownWorker request * Poll(Activity|Nexus|Workflow)TaskQueue requests This is "for merge". Here is a previous PR, where we discuss this at length: #599 <!-- Tell your future self why have you made these changes --> **Why?** Part of the worker visibility work. Workers should be able to report their status. Users should be able to get information about workers. <!-- Are there any breaking changes on binary or code level? --> **Breaking changes** Yes - new API is introduce. <!-- If this breaks the Server, please provide the Server PR to merge right after this PR was merged. --> **Server PR** [Server PR](temporalio/temporal#7870)
<!-- Describe what has changed in this PR --> **What changed?** What changed? * 2 new APIs added: * WorkerHeartbeat * ListWorkerStatus * WorkerInfo information added to: * ShutDownWorker request * Poll(Activity|Nexus|Workflow)TaskQueue requests This is "for merge". Here is a previous PR, where we discuss this at length: temporalio/api#599 <!-- Tell your future self why have you made these changes --> **Why?** Part of the worker visibility work. Workers should be able to report their status. Users should be able to get information about workers. <!-- Are there any breaking changes on binary or code level? --> **Breaking changes** Yes - new API is introduce. <!-- If this breaks the Server, please provide the Server PR to merge right after this PR was merged. --> **Server PR** [Server PR](temporalio/temporal#7870)
MAYBE FOR MERGE.
The goal of this PR is to review the proposed APIs.
It will not necessary be merged, treat it as a part of Design review.
What changed?
2 new APIs added:
Why?
Part of the worker visibility work.
Note:
Currently metrics are not supported as a part of List operation query. But it may happen, based on user feedback.