Skip to content

Add worker visibility API - heartbeat and list worker satus#599

Closed
ychebotarev wants to merge 10 commits into
masterfrom
y_heartbeat
Closed

Add worker visibility API - heartbeat and list worker satus#599
ychebotarev wants to merge 10 commits into
masterfrom
y_heartbeat

Conversation

@ychebotarev
Copy link
Copy Markdown
Contributor

@ychebotarev ychebotarev commented May 28, 2025

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:

  • WorkerHeartbeat
  • ListWorkerStatus

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.

Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/workflowservice/v1/service.proto Outdated
Comment thread temporal/api/workflowservice/v1/service.proto Outdated
Comment thread temporal/api/workflowservice/v1/service.proto Outdated
Comment thread temporal/api/worker/v1/message.proto
Comment thread temporal/api/workflowservice/v1/request_response.proto
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/workflowservice/v1/service.proto
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto
Comment thread temporal/api/workflowservice/v1/request_response.proto Outdated
Comment thread temporal/api/workflowservice/v1/request_response.proto Outdated
Comment thread temporal/api/workflowservice/v1/service.proto
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread 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;
Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keeping this open

Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/workflowservice/v1/service.proto
Comment thread temporal/api/workflowservice/v1/request_response.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
@ychebotarev ychebotarev requested review from cretz and dnr June 2, 2025 17:05
Copy link
Copy Markdown
Member

@Sushisource Sushisource left a 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.

Comment thread temporal/api/enums/v1/common.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
Comment thread temporal/api/worker/v1/message.proto Outdated
@ychebotarev ychebotarev requested a review from Sushisource June 3, 2025 02:31
@ychebotarev ychebotarev closed this Jun 4, 2025
ychebotarev added a commit that referenced this pull request Jun 5, 2025
<!-- 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)
temporal-cicd Bot pushed a commit to temporalio/api-go that referenced this pull request Jun 5, 2025
<!-- 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants