Prevent negative trigger queue delay metrics#69971
Conversation
System clock adjustments can make trigger startup latency negative or inflate it, producing misleading observability data. Signed-off-by: viiccwen <vicwen@apache.org>
shahar1
left a comment
There was a problem hiding this comment.
This re-applies the time.monotonic() change that was removed during review of #67927 (queued_at is stamped in the supervisor but subtracted in the runner subprocess).
Please either implement a more robust solution for this issue, or pivot this PR to adding a comment for why we use time.time() here.
|
Hello @shahar1, Although the reference point of On Linux, this is backed by For this metric, we are measuring an elapsed duration, so I see here: airflow/airflow-core/src/airflow/jobs/triggerer_job_runner.py Lines 272 to 278 in fe9d865 then here: airflow/airflow-core/src/airflow/jobs/triggerer_job_runner.py Lines 547 to 554 in fe9d865 so the relation is like: Triggerer host/container -> Triggerer main process -> TriggerRunnerSupervisor -> local TriggerRunner subprocess IMO, we can add a comment documenting the same-host and cross-process clock assumption, not just use |
Thanks for challenging my claim - it seems like you've got a point :) @SameerMesiah97 - FYI, I'll be happy for your feedback. |
closes #69970
Trigger queue delay is a duration measured from when the supervisor queues a workload until the runner creates its trigger. Using wall-clock timestamps for that duration can produce negative or inflated metrics when the system clock is adjusted.
This changes both ends of the measurement to use the monotonic clock. The regression tests simulate the wall clock moving backward and cover both the supervisor timestamp and the runner's emitted metric.
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.