Skip to content

Bound workflow task latency metric by wall-clock time in metrics test - #1839

Merged
tconley1428 merged 1 commit into
mainfrom
flake/metrics-latency-wallclock-bound
Sep 10, 2026
Merged

Bound workflow task latency metric by wall-clock time in metrics test#1839
tconley1428 merged 1 commit into
mainfrom
flake/metrics-latency-wallclock-bound

Conversation

@DABH

@DABH DABH commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

What was changed

test_workflow_buffered_metrics bounds workflow_task_execution_latency by the measured worker lifetime instead of a fixed 3s, in both millisecond and second modes.

Why

A stalled CI run took about 9s for the second workflow and failed the fixed < 3s bound. The wall-clock bound still distinguishes the two unit modes. #1819 touches a different test.

Testing

Emulated 4s stall: fails before, passes after. 240/240 flake-finder runs under load. Lint clean.

test_workflow_metrics_other_types asserted that the workflow task execution latency was under 3s (seconds format) and over 3ms (milliseconds format) as a sanity check of the duration unit. On a loaded CI runner the first workflow task of the HelloWorkflow run took over 3s, failing the seconds assertion ('assert False' from 'any(...)'). Bound the recorded latency by how long the worker actually ran instead, which still distinguishes the two duration formats without assuming a fast machine.
@DABH DABH added the skip-changelog PR changes do not require changelog updates label Sep 10, 2026
@DABH
DABH requested a balanced review from Copilot September 10, 2026 06:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused test-only change correctly preserves unit validation while accommodating stalled environments.

Pull request overview

Updates the buffered metrics test to avoid failures during slow CI runs.

Changes:

  • Measures worker lifetime using time.monotonic().
  • Bounds latency assertions by elapsed time for both duration formats.
File summaries
File Description
tests/worker/test_workflow.py Replaces fixed latency thresholds with wall-clock bounds.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@DABH
DABH marked this pull request as ready for review September 10, 2026 07:55
@DABH
DABH requested a review from a team as a code owner September 10, 2026 07:55
@tconley1428
tconley1428 merged commit 3ccff96 into main Sep 10, 2026
34 of 36 checks passed
@tconley1428
tconley1428 deleted the flake/metrics-latency-wallclock-bound branch September 10, 2026 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog PR changes do not require changelog updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants