Skip to content

Conversation

stephanos
Copy link
Contributor

@stephanos stephanos commented Jul 10, 2025

What changed?

Adding a breakdown of stats per-priority levels to the two APIs that return task queue stats already.

Why?

New API is defined here: temporalio/api#611.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

@stephanos stephanos changed the title Tq pri stats TaskQueue per-priority stats Jul 10, 2025
@stephanos stephanos changed the title TaskQueue per-priority stats TaskQueue per-priority stats in API Jul 10, 2025
@stephanos stephanos force-pushed the tq-pri-stats branch 6 times, most recently from c41265b to 4e63150 Compare July 10, 2025 04:41
@stephanos stephanos requested a review from dnr July 10, 2025 04:41
@stephanos stephanos marked this pull request as ready for review July 10, 2025 05:09
@stephanos stephanos requested a review from a team as a code owner July 10, 2025 05:09
@stephanos stephanos force-pushed the tq-pri-stats branch 17 times, most recently from 6b011d0 to b2cf130 Compare July 15, 2025 20:07
WorkflowTaskTimeout: durationpb.New(10 * time.Minute),
RequestId: uuid.New(),
WorkflowIdReusePolicy: enumspb.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE,
Priority: &commonpb.Priority{PriorityKey: int32(priority)},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a notable test update.

@@ -102,7 +118,7 @@ func (s *TaskQueueStatsSuite) TestMultipleTasks_MultiplePartitions_WithMatchingB
s.OverrideDynamicConfig(dynamicconfig.TaskQueueInfoByBuildIdTTL, 1*time.Millisecond) // zero means no TTL

s.RunTestWithMatchingBehavior(func() {
s.publishConsumeWorkflowTasksValidateStats(50, false) // 25 unversioned, 25 versioned
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed these from totals to "sets".

}, 5*time.Second, 100*time.Millisecond)
}

func validateTaskQueueStatsByPriority(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a net new set of assertions.

// Unless a test calls out a specific methods, all three methods are tested in each test case.
func TestTaskQueueStatsSuite(t *testing.T) {
// TODO(pri): remove once the classic matcher is removed
func TestTaskQueueStatsSuite_ClassicMatcher(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

the naming scheme for the others is:

Suggested change
func TestTaskQueueStatsSuite_ClassicMatcher(t *testing.T) {
func TestTaskQueueStats_Classic_Suite(t *testing.T) {

I don't care that much but just keep them easy to type and consistent

stephanos added a commit to temporalio/api that referenced this pull request Jul 22, 2025
_**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._

<!-- Describe what has changed in this PR -->
**What changed?**

Adding a breakdown of stats per-priority levels to the two APIs that
return task queue stats already.

<!-- Tell your future self why have you made these changes -->
**Why?**

To give users insights on a per priority level.

<!-- Are there any breaking changes on binary or code level? -->
**Breaking changes**

N/A

<!-- If this breaks the Server, please provide the Server PR to merge
right after this PR was merged. -->
**Server PR**

temporalio/temporal#8030
temporal-cicd bot pushed a commit to temporalio/api-go that referenced this pull request Jul 22, 2025
_**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._

<!-- Describe what has changed in this PR -->
**What changed?**

Adding a breakdown of stats per-priority levels to the two APIs that
return task queue stats already.

<!-- Tell your future self why have you made these changes -->
**Why?**

To give users insights on a per priority level.

<!-- Are there any breaking changes on binary or code level? -->
**Breaking changes**

N/A

<!-- If this breaks the Server, please provide the Server PR to merge
right after this PR was merged. -->
**Server PR**

temporalio/temporal#8030
@stephanos stephanos enabled auto-merge (squash) July 22, 2025 20:26
@stephanos stephanos merged commit f4b7865 into temporalio:main Jul 22, 2025
53 checks passed
@stephanos stephanos deleted the tq-pri-stats branch July 22, 2025 21:33
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.

2 participants