Making tq stats visible from cli #638

Shivs11 · 2024-08-19T20:24:59Z

What was changed

Adds support to view TQ stats from the CLI.
Sample input and output for the command:

temporal task-queue describe --task-queue xxx

Task Queue Statistics

BuildID	TaskQueueType	ApproximateBacklogCount	ApproximateBacklogAge	BacklogIncreaseRate	TasksAddRate	TasksDispatchRate
UNVERSIONED	workflow	0	0s	0	0	0
UNVERSIONED	activity	0	0s	0	0	0

temporal task-queue describe --task-queue xxx -o json

"stats": [ { "buildId": "UNVERSIONED", "taskQueueType": "workflow", "approximateBacklogCount": 0, "approximateBacklogAge": 0, "backlogIncreaseRate": 0, "tasksAddRate": 0, "tasksDispatchRate": 0 }, { "buildId": "UNVERSIONED", "taskQueueType": "activity", "approximateBacklogCount": 0, "approximateBacklogAge": 0, "backlogIncreaseRate": 0, "tasksAddRate": 0, "tasksDispatchRate": 0 } ]

Why?

Shall help in worker fleet management.

Checklist

Closes
How was this tested:

Modified an existing unit test
Ensured all other unit tests pass

Any docs updates needed?

Updated docs.

CLAassistant · 2024-08-19T20:25:05Z

All committers have signed the CLA.

CLAassistant · 2024-08-19T20:25:05Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Shivs11 · 2024-08-19T20:26:18Z

go.mod

@@ -15,7 +15,7 @@ require (
 	github.com/stretchr/testify v1.9.0
 	github.com/temporalio/ui-server/v2 v2.29.2
 	go.temporal.io/api v1.36.0
-	go.temporal.io/sdk v1.27.0
+	go.temporal.io/sdk v1.28.2-0.20240816033637-2a02c483658a


Upgraded sdk to point to the version where TQ stats were introduced in DescribeTaskQueueEnhanced (#1553)

This cannot be merged to main until it is a stable tag on the dependency

temporalcli/commands.taskqueue.go

temporalcli/commands.taskqueue_test.go

temporalcli/commands.gen.go

cretz

Can you show what the output of this command w/ stats for a normal task queue (no versioning) looks like in text and JSON form? Can just be as a comment here in the PR. It's hard to get a feel for what the user sees from the code alone.

cretz · 2024-08-20T12:34:52Z

go.mod

@@ -15,7 +15,7 @@ require (
 	github.com/stretchr/testify v1.9.0
 	github.com/temporalio/ui-server/v2 v2.29.2
 	go.temporal.io/api v1.36.0
-	go.temporal.io/sdk v1.27.0
+	go.temporal.io/sdk v1.28.2-0.20240816033637-2a02c483658a


This cannot be merged to main until it is a stable tag on the dependency

temporalcli/commands.gen.go

temporalcli/commands.taskqueue_test.go

temporalcli/commands.taskqueue.go

…ases

ShahabT · 2024-08-26T18:56:43Z

@antlai-temporal @Shivs11 I think it's better to change the short and long descriptions of the describe command. Right now it seems we're over-focusing on the reachability info. e.g. the short description is "Provides task reachability and pollers information for Workers on this Task Queue.".

I'd suggest we mention poller info and stats as the main output. Reachability can be mentioned as a experimental part of the API with proper warning that it may change in the future. Note that there is a chance that we want to move reachability API under temporal versioning command, and in that case it's not clear if we'd still keep it under temporal task-queue describe too.

Shivs11 · 2024-08-27T13:28:22Z

@antlai-temporal @Shivs11 I think it's better to change the short and long descriptions of the describe command. Right now it seems we're over-focusing on the reachability info. e.g. the short description is "Provides task reachability and pollers information for Workers on this Task Queue.".

I'd suggest we mention poller info and stats as the main output. Reachability can be mentioned as a experimental part of the API with proper warning that it may change in the future. Note that there is a chance that we want to move reachability API under temporal versioning command, and in that case it's not clear if we'd still keep it under temporal task-queue describe too.

I was not aware that the reachability side of things could be moved over later and knowing this now, it makes sense to not over-focus on it. I am for the act of keeping pollers and stats as the main output but shall wait for @antlai-temporal to give us the green light 🚥

temporalcli/commands.taskqueue.go

cretz · 2024-08-28T13:03:39Z

Can you show what the output of this command w/ stats for a normal task queue (no versioning) looks like in text and JSON form?

I saw the markdown table in PR description but I think we want to see what the raw table looks like in text (and will wait until after y'all's discussion on text/json format to review what latest output looks like and approve).

Shivs11 · 2024-08-28T15:04:35Z

Can you show what the output of this command w/ stats for a normal task queue (no versioning) looks like in text and JSON form?

I saw the markdown table in PR description but I think we want to see what the raw table looks like in text (and will wait until after y'all's discussion on text/json format to review what latest output looks like and approve).

Ah, my apologies for the misunderstanding. Pasting the output as seen from the CLI here:

Scenario - Adding a task in the backlog with no pollers:

JSON

Non-JSON

note - the backlog increase rate becomes 0 in the second picture because we are fetching the rate over a window of 30 seconds which were elapsed between the two screenshots.

cretz

LGTM, but would like to wait for other approvers because I think they may have stronger opinions on the output structure. And please don't merge until we have a tagged SDK dependency.

antlai-temporal · 2024-08-28T17:41:43Z

I was not aware that the reachability side of things could be moved over later and knowing this now, it makes sense to not
over-focus on it. I am for the act of keeping pollers and stats as the main output but shall wait for @antlai-temporal to give us the green light

The main reason to over focus on reachability was the feedback that it was hard to find which command would give us that info. I think that once we move reachaibility to versioning there will be no issue, but until we do, it makes sense to give it priority in the short/long description.

antlai-temporal · 2024-08-28T18:10:30Z

temporalcli/commandsmd/commands.md

+* `--disable-stats` (bool) - Disable task queue statistics.



for reachability we use report-reachability as opposed to disable-reachability because it is expensive, and we wanted to avoid calls.
Is getting the stats also expensive? In that case we should use also use report-stats...

Stats is not the cheapest thing (because it need partition fan-out) but still relatively cheap. I'm comfortable with having it enabled by default. wdyt @dnr ?

We may want to add caching for stats in the root partition in the future (if SDK starts to query it for poller scaling).

ShahabT · 2024-08-28T19:26:14Z

temporalcli/commandsmd/commands.md

+information for the requested versions and all task types, which can be used to safely retire Workers with old code versions, 
+provided that they were assigned a Build ID.


this part of the sentence is only about reachability and it should be removed.

I agree, however we should have the part : the requested versions and all task types.

All in all, the sentence should look like:
The temporal task-queue describe command provides poller information, backlog statistics and task reachability (experimental) information for the requested versions and task types.

task reachability (experimental) ~~information~~

"information" seems redundant?

temporalcli/commandsmd/commands.md

…"_" (#641) Also [Bring back nexus workflow view tests](83b70c2) now that the SDK is released. Note that since #638 was merged into the nexus branch, it is no longer in a releasable state since `go.mod` is referencing an unreleased Go SDK.

Making tq stats visible from cli

78f2df8

Shivs11 commented Aug 19, 2024

View reviewed changes

Shivs11 requested a review from ShahabT August 19, 2024 20:48

ShahabT reviewed Aug 19, 2024

View reviewed changes

temporalcli/commands.taskqueue.go Show resolved Hide resolved

temporalcli/commands.taskqueue_test.go Outdated Show resolved Hide resolved

temporalcli/commands.gen.go Outdated Show resolved Hide resolved

Shivs11 requested review from cretz and antlai-temporal August 19, 2024 21:06

Shivs11 marked this pull request as ready for review August 19, 2024 21:16

Addressed comments and cleaned up unit tests

2cd8dc1

cretz requested a review from a team August 20, 2024 12:34

cretz reviewed Aug 20, 2024

View reviewed changes

changed to --disable-stats with default value as false

c8a3059

Shivs11 commented Aug 20, 2024

View reviewed changes

temporalcli/commands.gen.go Show resolved Hide resolved

Shivs11 added 3 commits August 20, 2024 12:19

fix breaking unit-test

8a1af2b

trying to solve unit test breaking on windows

ee19bdc

restored unit test to how it was previously

9b4dcfa

Shivs11 requested a review from ShahabT August 20, 2024 17:15

ShahabT reviewed Aug 23, 2024

View reviewed changes

temporalcli/commands.taskqueue_test.go Outdated Show resolved Hide resolved

ShahabT reviewed Aug 23, 2024

View reviewed changes

temporalcli/commands.taskqueue.go Show resolved Hide resolved

Shivs11 and others added 7 commits August 26, 2024 09:35

Removed and made a stricter check

e85cd84

Corrected test

c381d96

fixing a bug in the test

1bbd4f1

removing validating BacklogIncreaseRate since it could be 0 in some c…

13c5815

…ases

Separating TqStats from Pollers

aa62070

Delete temporalcli/flake_tester.sh

b7b7117

Restored older comment

4ddb3be

Shivs11 requested a review from ShahabT August 26, 2024 15:48

ShahabT reviewed Aug 27, 2024

View reviewed changes

temporalcli/commands.taskqueue.go Outdated Show resolved Hide resolved

changed backlogAge type to string + made unit tests better

d94ac25

cretz approved these changes Aug 28, 2024

View reviewed changes

antlai-temporal reviewed Aug 28, 2024

View reviewed changes

Refactored and altered Long and Short descriptions for describe command

7271406

ShahabT reviewed Aug 28, 2024

View reviewed changes

PR feedback

200e6ca

Shivs11 requested a review from ShahabT August 28, 2024 22:28

ShahabT approved these changes Aug 29, 2024

View reviewed changes

Removed redundancy in description

99d6255

Shivs11 merged commit 0120a01 into nexus Aug 29, 2024
5 of 6 checks passed

Shivs11 deleted the shivam/cli-tq-stats branch August 29, 2024 17:37

bergundy mentioned this pull request Aug 29, 2024

Upgrade server with endpoint regexp validation to allow "-" and deny "_" #641

Merged

BrewTestBot mentioned this pull request Sep 17, 2024

temporal 1.1.0 Homebrew/homebrew-core#190910

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Making tq stats visible from cli #638

Making tq stats visible from cli #638

Shivs11 commented Aug 19, 2024 •

edited

Loading

CLAassistant commented Aug 19, 2024 •

edited

Loading

CLAassistant commented Aug 19, 2024

Shivs11 Aug 19, 2024

cretz Aug 20, 2024

cretz left a comment •

edited

Loading

cretz Aug 20, 2024

ShahabT commented Aug 26, 2024

Shivs11 commented Aug 27, 2024

cretz commented Aug 28, 2024

Shivs11 commented Aug 28, 2024

cretz left a comment •

edited

Loading

antlai-temporal commented Aug 28, 2024

antlai-temporal Aug 28, 2024

ShahabT Aug 28, 2024

ShahabT Aug 28, 2024

Shivs11 Aug 28, 2024 •

edited

Loading

ShahabT Aug 29, 2024

		information for the requested versions and all task types, which can be used to safely retire Workers with old code versions,
		provided that they were assigned a Build ID.

Making tq stats visible from cli #638

Making tq stats visible from cli #638

Conversation

Shivs11 commented Aug 19, 2024 • edited Loading

What was changed

Task Queue Statistics

Why?

Checklist

CLAassistant commented Aug 19, 2024 • edited Loading

CLAassistant commented Aug 19, 2024

Shivs11 Aug 19, 2024

Choose a reason for hiding this comment

cretz Aug 20, 2024

Choose a reason for hiding this comment

cretz left a comment • edited Loading

Choose a reason for hiding this comment

cretz Aug 20, 2024

Choose a reason for hiding this comment

ShahabT commented Aug 26, 2024

Shivs11 commented Aug 27, 2024

cretz commented Aug 28, 2024

Shivs11 commented Aug 28, 2024

cretz left a comment • edited Loading

Choose a reason for hiding this comment

antlai-temporal commented Aug 28, 2024

antlai-temporal Aug 28, 2024

Choose a reason for hiding this comment

ShahabT Aug 28, 2024

Choose a reason for hiding this comment

ShahabT Aug 28, 2024

Choose a reason for hiding this comment

Shivs11 Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

ShahabT Aug 29, 2024

Choose a reason for hiding this comment

Shivs11 commented Aug 19, 2024 •

edited

Loading

CLAassistant commented Aug 19, 2024 •

edited

Loading

cretz left a comment •

edited

Loading

cretz left a comment •

edited

Loading

Shivs11 Aug 28, 2024 •

edited

Loading